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 CORS headers to Read Only Gateway Default config #2778

Merged
merged 2 commits into from
Jun 15, 2016

Conversation

Kubuxu
Copy link
Member

@Kubuxu Kubuxu commented Jun 1, 2016

This will allow for weider use of local gateway from different sites.

For example: ipfs.pics could use JS to use either their global or user's local gateway.

I might work on piece of JS doing just that but I don't really like JS. 😄

License: MIT
Signed-off-by: Jakub Sztandera kubuxu@protonmail.ch

License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@whyrusleeping
Copy link
Member

I like this idea, but people more knowledgable on http should review this, cc @diasdavid @travisperson @dignifiedquire

@dignifiedquire
Copy link
Member

I would suggest adding a test using curl to ensure it works as expected. See here for some details http://stackoverflow.com/questions/12173990/how-can-you-debug-a-cors-request-with-curl

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 1, 2016

We have tests for CORS headers, this just changes the default.

https://github.com/ipfs/go-ipfs/blob/master/test/sharness/t0112-gateway-cors.sh

@dignifiedquire
Copy link
Member

then you should add a test without changing the config so we know the default is working as epxected

License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 1, 2016

I've modified the test.

@ghost
Copy link

ghost commented Jun 15, 2016

This LGTM, sorry for the long wait

@whyrusleeping whyrusleeping merged commit 16245db into master Jun 15, 2016
@whyrusleeping whyrusleeping deleted the feature/CORS-on-gateway-by-default branch June 15, 2016 16:59
@ghost
Copy link

ghost commented Jun 15, 2016

Also deployed to gateways

@jbenet
Copy link
Member

jbenet commented Aug 27, 2016

thanks for pushing for that test @dignifiedquire 👍

@jbenet
Copy link
Member

jbenet commented Aug 27, 2016

Was that test actually created? @Kubuxu ?

@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 27, 2016

The test case is the fact that we don't have to explicitly set those settings in sharness for tests to succeed. See removed config calls.
https://github.com/ipfs/go-ipfs/pull/2778/files#diff-e75071bd34c34a76f7762bfb253223f5L13

@jbenet
Copy link
Member

jbenet commented Aug 28, 2016

This is not a specific test case. if this fails, it will take a long time to debug because of those other things failing not being super clear. remember that people who are not you will be hit by this. there should be test case that tests this as the first one to fail so debugging + fixing is easy.

@WardCunningham
Copy link

WardCunningham commented Sep 18, 2016

Thank you for this.

Here is the federated wiki pull request that benefits from this change.
fedwiki/wiki-client#173

@ghost
Copy link

ghost commented Sep 18, 2016

@WardCunningham this change is part of v0.4.3-rc4: https://dist.ipfs.io/go-ipfs/v0.4.3-rc4

@Mithgol
Copy link

Mithgol commented Feb 19, 2017

I have upgraded to go-ipfs version 0.4.5 and then I still don't see Access-Control-Allow-Origin: "*" in response headers of (for example) http://127.0.0.1:8080/ipfs/QmYZUa2E1bsuN37fwzjcawop4wk1kdXxzRyiaGfjKfWM2T though I see such header at https://ipfs.io/ipfs/QmYZUa2E1bsuN37fwzjcawop4wk1kdXxzRyiaGfjKfWM2T without any hindrance.

Does it mean that changes in the default config don't automatically apply to earlier adopters of go-ipfs?

The chat log from fedwiki/wiki-client#173 seems to say they don't:

(the chat log)

Then what exactly should I do myself (and recommend to the other IPFS users) to make the necessary changes in the local configuration and enable CORS?

@Kubuxu
Copy link
Member Author

Kubuxu commented Feb 19, 2017

You can run:

ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Origin '["*"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Methods '["GET"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Headers '["X-Requested-With"]'

@Mithgol
Copy link

Mithgol commented Feb 20, 2017

Thanks, it has worked.

For future references, on Windows the equivalent commands are

ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Origin [\"*\"]
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Methods [\"GET\"]
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Headers [\"X-Requested-With\"]

@ghost
Copy link

ghost commented Feb 20, 2017

I still think we should add these to the default config, as suggested here

@Kubuxu
Copy link
Member Author

Kubuxu commented Feb 20, 2017

It is in default config.

laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Feb 25, 2022
…ault

Add CORS headers to Read Only Gateway Default config
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Feb 25, 2022
…n-gateway-by-default

Add CORS headers to Read Only Gateway Default config
laurentsenta pushed a commit to laurentsenta/kubo that referenced this pull request Mar 4, 2022
…n-gateway-by-default

Add CORS headers to Read Only Gateway Default config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants