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

Sec-WebSocket-Protocol header is private[http] #137

Open
akka-ci opened this Issue Sep 8, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@akka-ci
Collaborator

akka-ci commented Sep 8, 2016

Issue by analytically
Tuesday Dec 22, 2015 at 11:06 GMT
Originally opened as akka/akka#19258


Overriding the default RejectionHandler is tricky for UnsupportedWebsocketSubprotocolRejection because Sec-WebSocket-Protocol is private.

@akka-ci akka-ci added this to the http-backlog milestone Sep 8, 2016

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 8, 2016

Collaborator

Comment by ktoso
Tuesday Dec 22, 2015 at 18:36 GMT


// cc @sirthias @jrudolph

Technically apps are not supposed to create these headers that's why it's private I guess, but your use case makes sense hm.
We may need to do something special other than making it not private?

Collaborator

akka-ci commented Sep 8, 2016

Comment by ktoso
Tuesday Dec 22, 2015 at 18:36 GMT


// cc @sirthias @jrudolph

Technically apps are not supposed to create these headers that's why it's private I guess, but your use case makes sense hm.
We may need to do something special other than making it not private?

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 8, 2016

Collaborator

Comment by ktoso
Thursday May 19, 2016 at 00:22 GMT


I wonder If we have an opinion on this one, do you remember @sirthias @jrudolph?

Collaborator

akka-ci commented Sep 8, 2016

Comment by ktoso
Thursday May 19, 2016 at 00:22 GMT


I wonder If we have an opinion on this one, do you remember @sirthias @jrudolph?

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 8, 2016

Collaborator

Comment by ktoso
Thursday May 19, 2016 at 00:22 GMT


I guess I'd open up, if someone wants to do bad things they still can just create one via reflection

Collaborator

akka-ci commented Sep 8, 2016

Comment by ktoso
Thursday May 19, 2016 at 00:22 GMT


I guess I'd open up, if someone wants to do bad things they still can just create one via reflection

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 8, 2016

Collaborator

Comment by sirthias
Monday May 30, 2016 at 12:36 GMT


Sorry for the late reaction (was on vacation last week).
I also don't see a problem with publicizing the header. But I think it was @jrudolph who added the header model initially and applied the private[http] modifier.

Collaborator

akka-ci commented Sep 8, 2016

Comment by sirthias
Monday May 30, 2016 at 12:36 GMT


Sorry for the late reaction (was on vacation last week).
I also don't see a problem with publicizing the header. But I think it was @jrudolph who added the header model initially and applied the private[http] modifier.

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 8, 2016

Collaborator

Comment by jrudolph
Monday May 30, 2016 at 15:55 GMT


IIRC it was the conservative choice. We usually wouldn't want users to put their own WebSocket headers into requests or responses. On the other hand it is really nice to have all the headers available for unforeseen use cases. If we open them up, they should be marked somehow to be not used in normal requests or responses (unless you know what you are doing, i.e. probably reimplementing some part of akka http). If we want users to be able to actually override those headers with their own this would need to be supported in the implementation so that we don't output several potentially conflicting headers of the same kind.

Collaborator

akka-ci commented Sep 8, 2016

Comment by jrudolph
Monday May 30, 2016 at 15:55 GMT


IIRC it was the conservative choice. We usually wouldn't want users to put their own WebSocket headers into requests or responses. On the other hand it is really nice to have all the headers available for unforeseen use cases. If we open them up, they should be marked somehow to be not used in normal requests or responses (unless you know what you are doing, i.e. probably reimplementing some part of akka http). If we want users to be able to actually override those headers with their own this would need to be supported in the implementation so that we don't output several potentially conflicting headers of the same kind.

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 8, 2016

Collaborator

Comment by ktoso
Tuesday May 31, 2016 at 13:22 GMT


supported in the implementation so that we don't output several potentially conflicting headers of the same kind.

Hm, that's a good consideration I have not thought about, thanks!

~~I'm on the fence of the need of working on this issue immediately, however the immediate need as explained in the first ticket here does make sense, leaving open for someone to pick up.

Collaborator

akka-ci commented Sep 8, 2016

Comment by ktoso
Tuesday May 31, 2016 at 13:22 GMT


supported in the implementation so that we don't output several potentially conflicting headers of the same kind.

Hm, that's a good consideration I have not thought about, thanks!

~~I'm on the fence of the need of working on this issue immediately, however the immediate need as explained in the first ticket here does make sense, leaving open for someone to pick up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment