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 user-agent to default list of Access-Control-Allow-Headers #5138

Closed
sohkai opened this issue Jun 19, 2018 · 4 comments · Fixed by #5893
Closed

Add user-agent to default list of Access-Control-Allow-Headers #5138

sohkai opened this issue Jun 19, 2018 · 4 comments · Fixed by #5893
Assignees
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@sohkai
Copy link

sohkai commented Jun 19, 2018

Version information:

Mostly to do with public gateways, but issue persists on recent versions:

go-ipfs version: 0.4.13-
Repo version: 6
System version: amd64/darwin
Golang version: go1.9.2

Type:

Enhancement

Description:

When sending requests across CORS, it looks like newer browsers are asking for user-agent in Access-Control-Allow-Headers, and failing fetches without it, e.g.:

image

It would be really nice if user-agent was included by default in the CORS settings. Also, not sure if setting it twice is the same as a list, but it seems like it should be a list?

See a similar API issue (Github's)

@bpierre
Copy link

bpierre commented Jul 3, 2018

In the meantime, it should be possible to update the headers using the config file.

@ivan386
Copy link
Contributor

ivan386 commented Oct 14, 2018

@bpierre Yes it possible. But this setting must be set on all gateways by default.

Firefox block request by XMLHttpRequest to all public gateways without it.

@Stebalien
Copy link
Member

I've now merged ipfs/go-ipfs-config#15 so this will be added to the default config. However, due to how our config system works, it won't be applied to existing nodes so I'm not going to close this issue quite yet.

We should either:

  1. Add a migration to add this to people's config's.
  2. Move these "defaults" out of the config. That is, always apply them regardless of what the user requests. IMO, this may be the best solution as there's no reason to forbid these headers.

As one of our browser experts, thoughts @lidel?

@Stebalien Stebalien added the kind/bug A bug in existing code (including security flaws) label Nov 27, 2018
@lidel
Copy link
Member

lidel commented Nov 27, 2018

Ok with allowing for user-agent customization during fetch, we want to be compatible with vendor(s) following the spec (whatwg/fetch#37) 👍

As for the cleanup of defaults:

tl;dr I think we should go with (2) AND while at it make sure we will be able to do some cleanup described below.


I've looked at our headers recently and started tracking related issues at ipfs/in-web-browsers#132.

As you can see in "go-ipfs v0.4.18 defaults" section of ipfs/in-web-browsers#132 (comment) we already have "hardcoded" defaults for headers such as Access-Control-Expose-Headers.

The need to hardcode support for user-agent out of the box in -Allow-Headers is similar to having X-Chunked-Output in -Expose-Headers (go-ipfs returns it even if not present in config because "it is part of the stack" and without it we are unable to detect streaming responses in js-ipfs-api).

@Stebalien Note that -Allow-Headers and -Expose-Headers control different things (request vs response) and defaults for each should be hardcoded separately, so we can clean them up later (eg. X-Chunked-Output makes sense only in -Expose-Headers).

Stebalien added a commit that referenced this issue Jan 4, 2019
fixes #5138 -- always add user-agent to access-control-allow-headers.
fixes #5888 -- same with content-type.
fixes #5892 -- extend user-provided headers instead of overriding them.

License: MIT
Signed-off-by: Steven Allen <[email protected]>
@ghost ghost assigned Stebalien Jan 4, 2019
@ghost ghost added the status/in-progress In progress label Jan 4, 2019
Stebalien added a commit that referenced this issue Jan 4, 2019
fixes #5138 -- always add user-agent to access-control-allow-headers.
fixes #5888 -- same with content-type.
fixes #5892 -- extend user-provided headers instead of overriding them.

License: MIT
Signed-off-by: Steven Allen <[email protected]>
@ghost ghost removed the status/in-progress In progress label Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants