Support wildcard for ExposedHeaders option. #84
Conversation
Via echoing back all headers in a wrapped response writer since browsers don't currently support the wildcard. Fixes rs#79
| // ExposeAllRespWriter echos back any headers that are set in the wrapped response writer | ||
| // to support the wildcard "*" case for Access-Control-Expose-Headers since | ||
| // browsers do not currently have good compatibility. | ||
| type ExposeAllRespWriter struct { |
There was a problem hiding this comment.
Please do not wrap the response writer. This has the undesired side effect of hiding optional interfaces. Introducing this would silently break many code bases using package.
There was a problem hiding this comment.
Yes I have run into that many times, but never seen any good solutions. How do you suggest the headers that are set by the wrapped handler from cors.(*Cors).Handler() by caught to then be applied as they are in here?
There was a problem hiding this comment.
I don't think it's doable, but I'd rather give up on the feature than breaking many users of the lib.
There was a problem hiding this comment.
It is prevalent to wrap response writers for middleware of various kinds since there isn't another way to do this, and code that relies on optional interfaces to do a necessary thing (even necessary performance) seems like a bad idea in all the cases I have run across, so personally I would be hesitant to encourage that, but certainly it is a valid point.
If the caller has the opportunity to wrap themselves and make sure the interfaces they want are supported, would that be enough to ease the concern? I can add a field to the options of type http.ResponseWriter, and the caller can wrap the exported type here (rather than implementing it themselves), which also would make it an optional feature. However the argument could also be made that using AllowAll() is already optional ;)
There was a problem hiding this comment.
I do not agree. For some applications, those optional interfaces are the only way to go (think about flusher for instance). It is not the role of a library to encourage or discourage the way the http library is used. The lib must not break any application, or it will lose the trust of its users.
The support for wildcards with Access-Control-Expose-Headers will only increase. So I would prefer to just plan for that and not add tech dept to this lib.
Via echoing back all headers in a wrapped response writer since browsers
don't currently support the wildcard.
Fixes #79