com.sun.net.httpserver.Headers issues & optimizations

[Robert Engels]

Hi. 

As to 2., then the change needed is if the user passes null to the constructor it should avoid creating the internal map and the copy. Copying all of the values is even less efficient. 

The reason 3 is an issue is that static methods cannot be overridden, so having an api with an implementation that uses an internal class you can no longer do the “override all methods” to achieve how you said these changes could be accomplished.

I think you’ll agree that Headers should have been an interface - not sure how it passed code review. 

> On Jan 9, 2025, at 7:54 AM, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
> 
> Hello Robert,
> 
> On 07/01/25 12:05 am, robert engels wrote:
> > ...
> >
> > 2. Another proposed change to Headers would be to add a protected constructor that allowed you to pass in the Map implementation (or null), allowing specialized map data structures that are better suited to http headers storage, especially when being converted to/from http2 headers.
> >
> > Although it is possible to subclass Headers to provide your own map, you still pay a penalty because the base class will instantiate a map, and the code is quite ugly as you must override every method in the Headers class. You can see a sample implementation here https://github.com/robaho/httpserver/blob/main/src/main/java/robaho/net/httpserver/OptimizedHeaders.java
> >
> > The subclass does offer the ability to bypass the normalization completely with a package level method to be used internally by the server when it has already guaranteed the key is normalized.
> 
> I don't know if you have noticed, but starting Java 18, the Headers class already provides a (public) constructor which allows a Map to be passed in to the constructor https://docs.oracle.com/en/java/javase/23/docs/api/jdk.httpserver/com/sun/net/httpserver/Headers.html#%3Cinit%3E(java.util.Map). The implementation of the constructor however copies over the key/values into an internal Map of its own, so the passed Map (implementation) plays no further role after that point.
> 
> In my opinion, in its current form, the Headers class provides enough flexibility to allow for a subclass of Headers to provide a more efficient implementation if it wants to. Like you note, such a subclass needs to implement all the methods of java.util.Map interface to be able to properly provide that support, but it's still doable.
> 
> >
> > 3. The public static of() methods in Headers should be changed/removed, as they create a dependency on a sun internal class (sun.net.httpserver.UnmodifiableHeaders) from a public “api” class. These could be changed to instead wrap the map with an unmodifiable map.
> >
> 
> com.sun.net.httpserver.Headers is an API class and the method signature of com.sun.net.httpserver.Headers.of() is:
> 
> public static com.sun.net.httpserver.Headers of(Map<String,List<String>> headers)
> 
> The implementation of that method currently returns an instance of sun.net.httpserver.UnmodifiableHeaders which is an internal implementation class belonging to the same module. I don't follow why that's a bad thing - the API itself, through the method signature, doesn't expose any internal implementation class.
> 
> -Jaikiran