Add Access-Control-Expose-Headers

[zeripath]

Fix #12424

Signed-off-by: Andrew Thornton [email protected]

[silverwind]

Should probably only set them if cors.ENABLED is true.

[zeripath]

Does it matter? I mean it's still semantically useful?

[silverwind]

It shouldn't really matter but CORS-specific headers do not convey any meaning when not using CORS so I see them mostly as garbage information in such a scenario.

[silverwind]

Ideally we should check for the existance of the Origin request header to detect CORS requests and then set the Access-Control-* headers accordingly when its present. That way it will even work when CORS response headers were added by a reverse proxy (and not by ourselves) and we won't emit those useless headers on non-CORS requests.

[zeripath]

I think that detecting whether CORS is being set etc through use of the Origin header is likely to be error prone - I can't see a problem with just emitting these in any case.

Do we really need to add the additional x lines of code to detect if we think CORS is enabled and Origin is set correctly etc. (with all the risk of getting that wrong) in these 14 places to determine whether we strictly need to emit this header that does no harm if it's not strictly needed but does no harm if it's emitted otherwise?

[silverwind]

Origin will be present on POST requests so it'll have some false positives. I guess I don't really care if they are always set.

[codecov-commenter]

Codecov Report

Merging #12446 into master will increase coverage by 0.03%.
The diff coverage is 84.21%.

@@            Coverage Diff             @@
##           master   #12446      +/-   ##
==========================================
+ Coverage   43.73%   43.77%   +0.03%     
==========================================
  Files         631      631              
  Lines       69851    69870      +19     
==========================================
+ Hits        30552    30583      +31     
+ Misses      34345    34334      -11     
+ Partials     4954     4953       -1     

Impacted Files	Coverage Δ
modules/context/context.go	57.06% <0.00%> (-0.32%)	⬇️
routers/api/v1/admin/org.go	34.09% <0.00%> (-0.80%)	⬇️
routers/api/v1/user/repo.go	41.07% <50.00%> (+0.33%)	⬆️
modules/lfs/server.go	45.53% <100.00%> (+0.30%)	⬆️
routers/api/v1/admin/user.go	17.48% <100.00%> (+0.58%)	⬆️
routers/api/v1/org/org.go	61.11% <100.00%> (+0.43%)	⬆️
routers/api/v1/org/team.go	36.52% <100.00%> (+0.27%)	⬆️
routers/api/v1/repo/commits.go	71.61% <100.00%> (+0.18%)	⬆️
routers/api/v1/repo/issue.go	53.26% <100.00%> (+0.46%)	⬆️
routers/api/v1/repo/pull.go	25.36% <100.00%> (+0.15%)	⬆️
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e266ba3...ea94f7e. Read the comment docs.