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

API default headers still overwrite user ones (and other inconsistencies) #5909

Open
hsanjuan opened this issue Jan 9, 2019 · 0 comments
Open
Labels
topic/CORS Issues related to CORS on HTTP endpoints

Comments

@hsanjuan
Copy link
Contributor

hsanjuan commented Jan 9, 2019

Version information: master

Type: meta, doc, bug

Description:

I almost went crazy configuring CORS and custom Headers. Let's look at this:

  "API": {
    "HTTPHeaders": {
      "Access-Control-Allow-Credentials": [
        "true"
      ],
      "Access-Control-Allow-Origin": ["o1", "o2"],
      "Access-Control-Allow-Methods": ["GET", "POST"],
      "Access-Control-Expose-Headers": ["X-H1", "X-H2"],
      "Access-Control-Allow-Headers": ["X-H3, X-H4"],
      "My-Header":["v1", "v2"]
    }

Access-Control-Allow-Credentias is set with a for loop to make sense out of it: https://github.com/ipfs/go-ipfs/blob/42a15ba7e4437648e2429b599eaa1b3937f6811e/core/corehttp/commands.go#L65 Ok, should prevent the user from doing something too stupid.

Access-Control-Allow-Origin and Access-Control-Allow-Methods will result in a single header as set by the Cors helper library, with the different slice values joined by , .

Access-Control-Expose-Headers and Access-Control-Allow-Headers don't work in the API and I keep getting defaults with a single header for several values. In the Gateway, the same produces:

< Access-Control-Allow-Headers: Content-Type
< Access-Control-Allow-Headers: Range
< Access-Control-Allow-Headers: User-Agent
< Access-Control-Allow-Headers: X-H3, X-H4
< Access-Control-Allow-Headers: X-Requested-With
< Access-Control-Allow-Methods: GET
< Access-Control-Expose-Headers: Content-Range
< Access-Control-Expose-Headers: X-Chunked-Output
< Access-Control-Expose-Headers: X-H1
< Access-Control-Expose-Headers: X-H2
< Access-Control-Expose-Headers: X-Stream-Output

Other than having no way to un-expose the default-exposed headers (should it be possible?), this should be ok apparently (https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2). It's however inconsistent with how the API shows the default values and how the other CORS headers are handled (shouldn't they also be sent to the CORS library?).

Finally "My-Header":["v1", "v2"] results in two headers with the same name and different values, which is consistent with the Go Header object does, but inconsistent with how CORS come out of our APIs. If want a single header I can always do ["v1, v2"].

However I set "Access-Control-Allow-Methods" : ["m1,m2"], even when this would work for other headers, it will not work because CORS headers are special internally and they need to be an slice element per header value. So "POST, PUT" does not translate into ACA-Methods: POST, PUT. This was a bit counter-intuitive for me.

  • Should we split comma separated values v1, v2 before handling it to CORS ? Or just document that each element in the slice must be a single value (specifically for CORS)?
  • Should expose-headers and allow-headers be handled by the cors library (which will set them correctly depending on the request type by the way).
  • It seems that the API default headers still overwrite user given ones, but things are handled ok in the Gateway.
  • It seems that there is no way in the gateway to overwrite the default Expose/Allow-Headers values, only to append to them. Not sure the value of this, but also counter-intuitive, since other CORs headers can be overwritten completely.
  • It is not obvious that the gateway endpoint uses the API header configuration for api paths (/api/v0/). This makes it weird to configure the API and the Gateway as separate things for different users.
  • I was always of the impression that the IPFS API officially only takes POST requests (correct me if wrong), so allowing GET, PUT methods on API endpoints seems to normalize non standard interactions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/CORS Issues related to CORS on HTTP endpoints
Projects
None yet
Development

No branches or pull requests

2 participants