koajs / koa

Expressive middleware for node.js using ES2017 async functions
https://koajs.com
MIT License
35.11k stars 3.22k forks source link

req.origin should display the `origin` header if it exists, not the current hostname #1008

Open jonathanong opened 7 years ago

jonathanong commented 7 years ago

https://github.com/koajs/koa/blob/aaac09af1a6aa02161fead1422fac460fbdcce3e/lib/request.js#L95

what do you all think? this would be a breaking change

fengmk2 commented 7 years ago

origin header should use request.get('origin') better than request.origin.

fl0w commented 7 years ago

I think this would be a good change, request.origin isn't used by us for this reason. I think most plugins/users behind a proxy in need of origin get it via request.get anyway. E.g. https://github.com/koajs/cors/blob/master/index.js#L46

fl0w commented 7 years ago

If this is done (thus semver-major bump), maybe consider other #904, and dropping https://github.com/koajs/koa/blob/master/lib/application.js#L107?

@jonathanong if you'd like to proceed with a new version, maybe create a v3 milestone as "todos"?

edit oh, the milestone already existed!

jonathanong commented 7 years ago

👍

fl0w commented 7 years ago

What's the expected behaviour here?

if header.origin:
  return header.origin
else
  return this.protocol + this.host // current behaviour
iyuq commented 7 years ago

Don't agree with return header.origin when exist, else return protocol + host, For header.origin is mostly used for CORS. I think should return header.origindirectly.

broofa commented 5 years ago

origin should reflect exactly-and-only what exists in the request. Special casing the behavior will be confusing and unexpected, especially when proxies/load balancers/VPNs start getting involved.

qwelias commented 5 years ago

Yeah, it was very confusing to me, had to fallback to request.get('origin')

pke commented 4 years ago

So let me try to understand that. If I want to compose links in my response, that the client can directly follow I have to use the origin header? Is it always guaranteed to be set?

cleverboy32 commented 4 years ago

why i use baseURl 127.0.0.1:port/xxxxxxxx in ssr get data, get hostname is { localhost: port } just equal the url ..........

trainto commented 4 years ago

Don't agree with return header.origin when exist, else return protocol + host, For header.origin is mostly used for CORS. I think should return header.origindirectly.

I agree. origin just should indicate where a fetch originates from. Not the host to which the request is being sent.

Banou26 commented 3 years ago

How is this still not fixed, it's such a little change...

willmac997 commented 2 years ago

This is a problem for me to

siakc commented 8 months ago

It is a trivial change why help is wanted? May I help?

MarcGodard commented 8 months ago

@siakc please help... this is above my ability, but also need this.