Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to disable CORS proxy #71

Closed
juhana opened this issue Jul 27, 2020 · 11 comments
Closed

Add an option to disable CORS proxy #71

juhana opened this issue Jul 27, 2020 · 11 comments
Labels

Comments

@juhana
Copy link
Contributor

juhana commented Jul 27, 2020

Parchment automatically uses a proxy to download the story file if it detects that it's not on the same domain. There are two cases where the proxy makes things worse rather than better:

  1. The file is on a different domain, but the server sets the correct CORS headers so it works without the proxy. The file might be inaccessible to the proxy (localhost, internal network etc.)
  2. The URL is a data URL (data:...)

I've been able to get around it with default_story: "data:text/javascript,processBase64Zcode('....'); // .js" which tricks Parchment into thinking that it's a legacy .js file, but that's a very hacky solution.

Arguably case 2 is a bug and it should detect data URLs, but for case 1 we'd need either that setting the proxy URL option to null would disable it altogether, or a separate setting that disables the proxy. Preferably with options auto-detection (current behavior), always enabled or always disabled. HugoJS has this setting:

// use a CORS proxy to load game files?
// "always" (or true), "never" (or false), or "auto".
// The "auto" option uses proxy only if the story URL points to another domain.
use_proxy: "auto"
@curiousdannii
Copy link
Owner

I'll keep this in mind for the rewrite I was intending to start tomorrow ;).

I wonder how common the CORS headers are now. I also don't remember what happens if you request something you can't access, if the browser will actually download the whole file (which could be quite large) only to raise an error later.

What's the use case for a data URL?

@juhana
Copy link
Contributor Author

juhana commented Jul 27, 2020

Thanks! I would consider disabling the proxy an "I know what I'm doing" move so whoever does it would be responsible for any side effects that might occur.

What's the use case for a data URL?

I have Parchment running in an iframe where the main app passes the story file from memory so there's no actual URL it could load.

@curiousdannii
Copy link
Owner

curiousdannii commented Jul 27, 2020

Probably the best way would be to just pass in an arraybuffer/uint8array. I'll have a think about whether that should be an overloaded option for default_story or a separate option.

@curiousdannii
Copy link
Owner

curiousdannii commented Nov 23, 2021

@juhana Sorry it's taken so long.

I remember you saying that you'd like to be able to just pass in a data array, is that still the case? Would that be instead of using a data URL, or would you also need support for a data URL?

I'm now thinking there will be a parchment_options.auto_launch option to disable autolaunching, and then a function like parchment.load_data("zcode", Uint8Array(...)) which you could use to specify the story format and pass in the data file directly. Do you think this would fit what you want to do with it?

In regards to the proxy server, I'm thinking that it would just try loading the URL, and only if it fails would it try the proxy server. Do you think it really needs an option if that's how it works?

@juhana
Copy link
Contributor Author

juhana commented Nov 24, 2021

I remember you saying that you'd like to be able to just pass in a data array, is that still the case? Would that be instead of using a data URL, or would you also need support for a data URL?
I'm now thinking there will be a parchment_options.auto_launch option to disable autolaunching, and then a function like parchment.load_data("zcode", Uint8Array(...)) which you could use to specify the story format and pass in the data file directly. Do you think this would fit what you want to do with it?

Sounds good! No need for a data URL if a data array can be passed in.

In regards to the proxy server, I'm thinking that it would just try loading the URL, and only if it fails would it try the proxy server. Do you think it really needs an option if that's how it works?

That system would work for my use cases, but if it's certain that the proxy won't work (e.g. localhost URLs) it'd be cleaner if you could just tell it to not even bother trying the proxy.

@curiousdannii
Copy link
Owner

curiousdannii commented Nov 24, 2021

I implemented the above as parchment.load:

// Overloaded load
// engine can be a engine object or id
// story can be a path or Uint8Array
async load(format, story) {

I did change it so that it will try all URLs without the proxy, it no longer assumes other domains don't have proper CORS support. I guess I could add an option to disable the proxy (perhaps by setting the proxy_url to null?) but I doubt it would be useful. If you're in a position where you know the proxy won't work then can't you also ensure the url is correct? Actually, even without changing the code, simply setting the proxy_url to an invalid domain should in effect mean it is never used. It's such a niche use case that I think that might be enough.

@juhana
Copy link
Contributor Author

juhana commented Nov 24, 2021

If you're in a position where you know the proxy won't work then can't you also ensure the url is correct?

In theory yes, but as long as you can pass the URL as a query parameter you can always have old links pointing to deleted files.

If setting the proxy url to null would disable the proxy, that would be the most intuitive way for it to work. (It's the first thing I tried when I tried to disable it.)

@curiousdannii
Copy link
Owner

curiousdannii commented Nov 24, 2021

As it is now, I think setting it to null won't work, but setting it to "http://fake.invalid/" would. It would still call fetch(), it would just instantly fail.

@curiousdannii
Copy link
Owner

If you set

parchment_options = {auto_launch: 0}

Then Parchment won't launch automatically. You can then call parchment.load() passing a Uint8Array and a format name:

parchment.load(data, 'glulx')

I think your concerns here have basically been handled by now, but if you have any more just make a new issue.

@curiousdannii
Copy link
Owner

It's probably not necessary, but I've added a use_proxy option to disable use of the proxy.

@juhana
Copy link
Contributor Author

juhana commented Jun 30, 2022

Thanks, that'll certainly make things more straightforward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants