search

jetty-project / jetty-alpn

Implementation of ALPN (Application Layer Protocol Negotiation) Specification for OpenJDK 7 or greater
48 stars 27 forks source link

Backporting fix from d6eefa73e77632481c77ed2cd204940d56c1fca0 #6

Closed Scottmitch closed 9 years ago

Scottmitch commented 9 years ago

Issue https://github.com/jetty-project/jetty-alpn/issues/5 was fixed only for the latest code. alpn-boot versions are tightly coupled to JDK versions, and so anyone not using the latest JDK version will not get the benefits of the fix. It would be nice to maintain a branch pre jdk8_u25-b17 specifically for alpn-boot issues to account for this scenario.

Scottmitch commented 9 years ago

@sbordet - Ping.

sbordet commented 9 years ago

@Scottmitch maintaining these branches will result in a combinatorial number of branches to maintain, so I am not sure it is worth the pain. Also, fixing a ALPN issue on a JDK release that is deemed as vulnerable by Oracle leaves me doubts as well. I am inclined to not backport this fix to older JDK versions. If you really need it, Webtide provides commercial support.

Scottmitch commented 9 years ago

@sbordet - Not that I necessarily need it but this is not a JDK bug...its an alpn-boot bug. If this were part of the JDK then that is the end of that, but instead we have the opportunity to correct the buggy code.

maintaining these branches will result in a combinatorial number of branches to maintain

When a new alpn-boot bug is found this will require that you have 1 branch the at each commit before the upstream JDK changes were taken. These branches will only ever change if there is a new alpn-boot bug found. I'm not sure what the combinatorial is in reference to but I think the number of branches it is upper bounded by the number of upstream JDK updates (related to the area that alpn-boot overrides).

Also, fixing a ALPN issue on a JDK release that is deemed as vulnerable by Oracle leaves me doubts as well.

I don't necessarily disagree that people should be upgrading their JDKs but I'm not sure than an alpn-boot bug should be part of the reason.

Webtide provides commercial support.

Thanks but I don't think that will be necessary :) These changes may also help prevent others from having to seek commercial support for a bug that is within our control (and already patched).

Scottmitch commented 9 years ago

@sbordet - Are you still of the opinion that alpn-boot versions that do not correspond to the latest OpenJDK related code are "deprecated" and in a sense "use at your own risk"? Just to clarify this decision is primarily based on reducing maintenance overhead/work (as opposed to the preferred technical approach)? If so can we please make the "deprecated" versions more clear (on Jetty's ALPN version listing, and also in the README of this project)?

sbordet commented 9 years ago

@Scottmitch it's not my opinion, it's Oracle's. There was a bug, it's fixed. The fix requires the latest JDK. If I have time I'll try to look at this, but don't hold your breath.

Scottmitch commented 9 years ago

@sbordet - I interpreted our previous discussion https://github.com/jetty-project/jetty-alpn/issues/5#issuecomment-65241393 to mean was isolated to alpn-boot. If this was an incorrect interpretation I apologize. To be clear is this isolated to alpn-boot or not? If it is not alpn-boot specific then would you expect the unit tests after the backport to pass (on an older JDK 1.8.0_20)? I don't remember changing the unit tests as part of the backport.