Skip to content

Conversation

alessio-perugini
Copy link
Contributor

SSIA

@alessio-perugini alessio-perugini marked this pull request as ready for review December 18, 2023 08:45
@alessio-perugini alessio-perugini linked an issue Dec 18, 2023 that may be closed by this pull request
3 tasks
@alessio-perugini alessio-perugini self-assigned this Dec 18, 2023
@alessio-perugini alessio-perugini added the type: enhancement Proposed improvement label Dec 18, 2023
Copy link

@rhpco rhpco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it fixes the problem:

$ curl -X OPTIONS 'http://127.0.0.1:8991/pause' -H 'Origin: https://app.arduino.
cc' -H 'AllowPrivateNetwork: true' -v
*   Trying 127.0.0.1:8991...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8991 (#0)
> OPTIONS /pause HTTP/1.1
> Host: 127.0.0.1:8991
> User-Agent: curl/7.68.0
> Accept: */*
> Origin: https://app.arduino.cc
> AllowPrivateNetwork: true
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 204 No Content
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Headers: Origin,Authorization,Content-Type
< Access-Control-Allow-Methods: PUT,GET,POST,DELETE
< Access-Control-Allow-Origin: https://app.arduino.cc
< Access-Control-Allow-Private-Network: true
< Access-Control-Max-Age: 50
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< Date: Mon, 18 Dec 2023 16:13:29 GMT
< 
* Connection #0 to host 127.0.0.1 left intact

I've also tried with:
curl -X POST 'http://127.0.0.1:8991/pause' -H 'Origin: https://test.arduino.cc' -v and it gives a 403. Whereas with the previous implementation, it was returning a 200.. In either cases it seems that with the wrong origin the request is not processed at all. I don't think that's a problem.

@codecov-commenter
Copy link

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (fb135b0) 20.94% compared to head (e55c67e) 20.93%.

Files Patch % Lines
main.go 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #873      +/-   ##
==========================================
- Coverage   20.94%   20.93%   -0.02%     
==========================================
  Files          43       43              
  Lines        3151     3153       +2     
==========================================
  Hits          660      660              
- Misses       2395     2397       +2     
  Partials       96       96              
Flag Coverage Δ
unit 20.93% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alessio-perugini alessio-perugini merged commit ef2f8d0 into main Dec 18, 2023
@alessio-perugini alessio-perugini deleted the cors-private-network branch December 18, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement Proposed improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preflight to /upload is blocked by Chrome v104+

4 participants