-
Notifications
You must be signed in to change notification settings - Fork 21
Extending the get and set methods to get from response and set to request #10
Comments
I might need to add that this feature makes no sense when working with HttpServletRequest and HttpServletResponse. But your code only depends on request.getHeader and response.setHeader so I would not use HttpServletRequest and HttpServletResponse but more generic Request and Response interfaces that require a getHeader and setHeader method. I am intending to use this Cookie in a proxy and I am willing to do the necessary modifications. Do you think they are useful for your library as well? |
Hi, thanks for opening an issue. Where is the Java Cookie was intended for server-side, but since this is not JS and code size doesn't matter, I am not entirely against making it work for other purposes. I am just worried because things like this have the potential to violate the Single Responsibility Principle. Even if the change makes sense under Java Cookie context, I wouldn't add a new boolean flag to the existing methods, that will unnecessarily increase the Cyclomatic Complexity of the the whole execution. Instead of a boolean flag, we can try to abstract the container where it retrieves and set the cookie, something like what you suggested. Maybe remove the servlet dependency altogether using different strategies for getting and setting the cookie, or keeping the dependency but making servlet the default strategy. Anyway, I need to understand the use case better to see if it belongs to the intent of the project. |
HttpCookie: My personal use-case is as follows: I have a proxy that reads HTTP requests AND responses. I should be able to pseudocode:
This whole design using a boolean and a requestResponse object is This is my use-case. Another use case is the one where Java-Cookie replaces I would also work with generic Request and Response interfaces that have 2015-12-10 2:59 GMT+01:00 Fagner Brack [email protected]:
|
The more I am trying to generate before/after code to support my usecase, the more I realize it's not that easy. I should somehow extract the setting and getting of the cookies from the rest of the logic. But there are also differences between the 'Set-Cookie' and 'Cookie' header. For instance, the 'cookie' header doesn't have an AttributesDefinition. So remove(name, attributes) wouldn't make any sense. Maybe I should just build what I need instead of full blown cookie parser. I just feel that there should be some code reuse between the three usecases: client-side, server-side, proxy. |
We can abstract the encoding/decoding algorithm and publish as public classes. The I really thought about this when I was designing java-cookie, but then I didn't do it (even with lower class visibility) to prevent potential overengineering for a use-case that might not exist. I wonder if it makes sense creating a different artifact for the decoding/encoding algorithm and use it as dependency. Unfortunately that would assume who is using this is using through Maven. Can you confirm if there is any package manager in Java land that retrieves an artifact straight from the github repository (like what Bower does in the front-end)? Unfortunately I am not too familiarized with the Java ecossystem... |
Neither am I. I'm not entirely sure where you are going with this. By Java cookie is not available via a package manager atm either? 2015-12-10 17:41 GMT+01:00 Fagner Brack [email protected]:
|
Yes, that is the only thing that is shareable between different projects
I am not saying that the encoding/decoding algorithm should be retrieve using a package manager instead of using git. My hypothesis would be to have the encoding algorithm in a different artifact, following the npm trend of having one module doing only one thing and all other modules depending on it.
|
About the second bullet point, I am tending to conclude that the expectation might be that Java Cookie does everything "cookie-related" in Java, because the name implies Java Cookie - A simple Java API for handling cookies. It's not like Java Cookie - A simple Java API for handling cookies in the server. |
I see your point on but am still unclear on how to extract the encoding and How could we deal with that difference? 2015-12-11 15:52 GMT+01:00 Fagner Brack [email protected]:
|
We extract by extracting it and making it publicly available to be used elsewhere, what is not clear about that? Below are some examples using pseudo-code that probably better express what I mean: Add a new strategy for the current API
Use the algorithms elsewhere if you must MyCustomClientSideAPIOutsideJavaCookie {
getCookie( String key ) {
String value = getValueFromKey( key );
value = new RFC6265Decoding().decode( value );
return value;
}
}
Despite if you remove a cookie in the client or in the server, both need to be set with the same attributes, that's a rule in the browser land at least. I assume this is also necessary in a cookie handling API built using Java that will set cookies like the client. Could you elaborate why it doesn't make sense? |
If create a cookie from the 'cookie' header, no attributes are available 2015-12-11 19:14 GMT+01:00 Fagner Brack [email protected]:
|
When reading a cookie from the If it doesn't need to be aware of the path, domain and protocol of the request, then this definitely means we need to consider a new API for the client that is completely unrelated to In that case it might make sense abstracting the decoding/encoding mechanism in order to integrate with that new API. |
@roelstorms Is there any progress on this? |
Not yet, I am currently working on something else and I hope to come back 2015-12-19 13:20 GMT+01:00 Fagner Brack [email protected]:
|
I am still kind of struggling to understand the final result of the proposal. Can you post a snippet of code that you would be using to be able to set the cookies without using another custom framework on top of oficial Java technologies? Maybe that will make things clearer. The snippet you posted above was showing how you would do it using Java Cookie with the desired API using pseudo code, what I would need is the actual code you are using now without Java Cookie, if possible something I could run. Then it would be easier to think on something to abstract on top of Java Cookie or have some concrete arguments to justify not implementing it. |
I was still working on a design so I can't show you a working piece of I have an incoming response which is a sequence of bytes. Burpsuite allows But I also need a cookie library. I would like to parse all the set-cookie This would look something like this: // This is a BurpSuite API method for intercepting messages that pass Response response = new MyResponse(message.getMessageInfo()); Cookies cookies = Cookies.initFromResponse(response); String sessionCookie = cookies.get('PHPSESSIONID'); store(response, sessionCookie); cookies.remove('PHPSESSIONID');
} else{ ... } } For the requests I would like to do something similar. I would like to public void processProxyMessage(boolean messageIsRequest, ... } else{ Request request = new MyResponse(message.getMessageInfo()); Cookies cookies = Cookies.initFromRequest(request);
} } interface Request{ interface Response{ I hope this makes my usage clearer? But I wouldn't want to restrict it to It could also be possible to use it at the client side (replacing Ps: How are you dealing with requests that have multiple 'cookie' headers. public java.lang.String getHeader(java.lang.String name) I would rather use and make Cookies support the following interface: interface Request{ 2015-12-21 17:23 GMT+01:00 Fagner Brack [email protected]:
|
That was not something that I accounted for, I assumed a single cookie request header, which is what browsers do in the wild. I will create an issue to track and research about that. EDIT: |
Note: Please format you code using I can only say that decoupling the encoding/decoding mechanism (which is RFC 6165 compliant) would have a clear value for different use cases, not just for yours specific. I can see the value of not reinventing the wheel and use an encoding mechanism that already considers the RFC 6265. I will be waiting for a PR in order to understand the use case better, in the meantime I have added an issue to decouple the encoding/decoding mechanism from the core in order to make it public if necessary. |
In Cookies.java the set(String name, String Value, AttributesDefinition attributes);
will set the cookie using setCookie(String cookieValue, HttpServletResponse response); which will use the 'Set-Cookie' header.
This is for the usecase when using this library at the server side. When using this Cookie at client side (as a replacement for HttpCookie) it would be useful to set cookies to the Request and get cookies from the Response. So the other way around from how it is now.
It would ask for two setCookie methods, one using Request and one using Response as a parameter. The set(String name, String Value, AttributesDefinition attributes); method could be extended with a boolean like this:
set(String name, String Value, AttributesDefinition attributes, boolean setForRequest);
Of course the value of the cookie should also be modified since it shouldn't contain all the attributes when written to a Request.
The same can be done for get() -> get(boolean fromRequest)
The parsing algorithm should be modified to parse all the parameters that could be carried in the 'set-cookie' header.
This modification would allow this Cookie class to be used server-side, client-side and in a proxy.
The text was updated successfully, but these errors were encountered: