Skip to content
This repository has been archived by the owner on Feb 8, 2023. It is now read-only.

Extending the get and set methods to get from response and set to request #10

Open
roelstorms opened this issue Dec 8, 2015 · 18 comments

Comments

@roelstorms
Copy link

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.

@roelstorms
Copy link
Author

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?

@FagnerMartinsBrack
Copy link
Member

Hi, thanks for opening an issue.

Where is the HttpCookie you are referring to from? Can you provide a code example showing what it would be replacing (before/after)? I just want to understand the use case.

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.

@roelstorms
Copy link
Author

HttpCookie:
https://docs.oracle.com/javase/7/docs/api/java/net/HttpCookie.html
It is basically the client-side version of you Java-Cookie but it doesn't
support the newest specification which has been an open issue since 2012:
https://java.net/jira/browse/SERVLET_SPEC-37

My personal use-case is as follows:

I have a proxy that reads HTTP requests AND responses. I should be able to
add, modify, read and remove cookies from the request and response.

pseudocode:
/*

  • Depending on which message is intercepted either the request or response
    is null
    */
    intercept(RequestResponse requestResponse, boolean isRequest){
    if(isResponse){
    Request request = requestResponse.getRequest();
    Cookies cookies = Cookies(request);
    // Do some processing on the cookies
    requestResponse.setRequest(request);
    }
    else{
    Response response = requestResponse.getResponse();
    Cookies cookies = Cookies(response);
    // Do some processing on the cookies
    requestResponse.setResponse(response);
    }
    }

This whole design using a boolean and a requestResponse object is
pseudocode resembling BurpSuites' API. It's probably not the cleanest OO
design and I don't agree with it either.

This is my use-case. Another use case is the one where Java-Cookie replaces
the net.HttpCookie since it hasn't been following the spec for years. In
that case you would create a Cookie from response.getHeader() and set a
cookie with request.setHeader().

I would also work with generic Request and Response interfaces that have
the getHeader and setHeader methods so that you don't depend on
HttpServletRequest and HttpServletResponse. But you seem far more
experiences in OO design so I might be missing some crucial design criteria
here.

2015-12-10 2:59 GMT+01:00 Fagner Brack notifications@github.com:

Where is the HttpCookie you are referring to from? Can you provide a code
example showing what it would be replacing (before/after)? I just want to
understand the use case.

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
https://en.wikipedia.org/wiki/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
https://en.wikipedia.org/wiki/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.

@roelstorms
Copy link
Author

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.

@FagnerMartinsBrack
Copy link
Member

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 Cookies interface wouldn't change and you can be able to use it in whatever operation you like.

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...

@roelstorms
Copy link
Author

Neither am I. I'm not entirely sure where you are going with this. By
encoding/decoding algorithm you mean extracting the encode and decode
methods? Why should they be retrieved using a package manager opposed to
using git directly?

Java cookie is not available via a package manager atm either?

2015-12-10 17:41 GMT+01:00 Fagner Brack notifications@github.com:

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 Cookies interface wouldn't change and you can be able to use
it in whatever operation you like.

I really thought about this when I was designing java-cookie, but then I
didn't do it (even with package 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.


Reply to this email directly or view it on GitHub
#10 (comment)
.

@FagnerMartinsBrack
Copy link
Member

By encoding/decoding algorithm you mean extracting the encode and decode
methods?

Yes, that is the only thing that is shareable between different projects

Why should they be retrieved using a package manager opposed to
using git directly?

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.

  • The problem is that it will only be a benefit for those who use a package manager (Maven) because otherwise you would have to add the dependency by yourself (or I can release an artifact on Github with all dependencies).
  • Another potential problem is the expectation: Is it expected that those things are broken in different artifacts or one expect Java Cookie to be a monolithic general solution for cookie handling in Java?

@FagnerMartinsBrack
Copy link
Member

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.

@roelstorms
Copy link
Author

I see your point on but am still unclear on how to extract the encoding and
decoding from Cookies. It's not just the encoding/decoding that changes but
also the validity of attributes. As I mentioned earlier, it doesn't make
sense to call remove(name, attributes) on a cookies object that uses the
'cookie' header as encoding format.

How could we deal with that difference?

2015-12-11 15:52 GMT+01:00 Fagner Brack notifications@github.com:

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
.


Reply to this email directly or view it on GitHub
#10 (comment)
.

@FagnerMartinsBrack
Copy link
Member

I see your point on but am still unclear on how to extract the encoding and decoding from Cookies

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

Encoding rfc6265 = new RFC6265Encoding();
Decoding rfc6265 = new RFC6265Decoding();
Cookies cookies = Cookies.initFromServlet( ... );
cookies.setEncodingMechanism( rfc6265 );

Use the algorithms elsewhere if you must

MyCustomClientSideAPIOutsideJavaCookie {
  getCookie( String key ) {
    String value = getValueFromKey( key );
    value = new RFC6265Decoding().decode( value );
    return value;
  }
}

it doesn't make sense to call remove(name, attributes) on a cookies object that uses the 'cookie' header as encoding format.

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?

@roelstorms
Copy link
Author

If create a cookie from the 'cookie' header, no attributes are available
since this header doesn't carry attributes. So you will just set them to
defaults then?

2015-12-11 19:14 GMT+01:00 Fagner Brack notifications@github.com:

I see your point on but am still unclear on how to extract the encoding
and decoding from Cookies

We extract by extracting it and making it publicly available to be used
elsewhere, what is not clear about that?

Add a new strategy for the current API

Encoding rfc6265 = new RFC6265Encoding();
Decoding rfc6265 = new RFC6265Decoding();
Cookies cookies = Cookies.initFromServlet( ... );
cookies.setEncodingMechanism( rfc6265 );

Use the algorithms elsewhere if you must

MyCustomClientSideAPIOutsideJavaCookie {
getCookie( String key ) {
String value = getValueFromKey( key );
value = new RFC6265Decoding().decode( value );
return value;
}
}

it doesn't make sense to call remove(name, attributes) on a cookies object
that uses the 'cookie' header as encoding format.

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?


Reply to this email directly or view it on GitHub
#10 (comment)
.

@FagnerMartinsBrack
Copy link
Member

When reading a cookie from the Cookie header, it contains no attributes, therefore it would be necessary to have knowledge of the path, domain and protocol of the request to be able to create a relationship between the Path, Domain and Secure attributes of the cookie to remove it.

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 CookiesDefinition interface.

In that case it might make sense abstracting the decoding/encoding mechanism in order to integrate with that new API.

@FagnerMartinsBrack
Copy link
Member

@roelstorms Is there any progress on this?

@roelstorms
Copy link
Author

Not yet, I am currently working on something else and I hope to come back
to the java Cookie within the next few weeks. If you have a design in mind
I would like to implement it. At the moment I am not sure how to properly
design a solution that works and is a proper OO design.

2015-12-19 13:20 GMT+01:00 Fagner Brack notifications@github.com:

@roelstorms https://github.com/roelstorms Is there any progress on this?


Reply to this email directly or view it on GitHub
#10 (comment)
.

@FagnerMartinsBrack
Copy link
Member

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.

@roelstorms
Copy link
Author

I was still working on a design so I can't show you a working piece of
code. If you aren't familiar with Burpsuite API, posting the code won't be
useful either. But I will try to give a more detailed explanation.

I have an incoming response which is a sequence of bytes. Burpsuite allows
me to do some basic processing to extra some information in very unusable
formats. For example, I can get the headers of the response but these are
returned as a List where each entry in the list is just a line of
the header. This is slightly irritating, since it also contains the
startline (GET someresource.html HTTP/1.1). It is also unusable since it
can't process headers spanning multiple lines. There is also no library to
extract cookies from the request of from this headers (List<String) object.
So I need some extra logic. Retrieving and setting headers I will implement
in a custom Request and Response class (interfaces found below).

But I also need a cookie library. I would like to parse all the set-cookie
headers I find in the List headers object to a list of cookies. I
would then search for a session cookie (a cookie containing a session ID,
not a cookie that is kept in cache) and remove it from the response to
store it in a database or in cache.

This would look something like this:

// This is a BurpSuite API method for intercepting messages that pass
through the proxy;
public void processProxyMessage(boolean messageIsRequest,
IInterceptedProxyMessage message){
if(!messageIsRequest){

Response response = new MyResponse(message.getMessageInfo());

Cookies cookies = Cookies.initFromResponse(response);

String sessionCookie = cookies.get('PHPSESSIONID');

store(response, sessionCookie);

cookies.remove('PHPSESSIONID');

        byte[] modifiedResponse = response.toBytes();
        message.getMessageInfo().setResponse(modifiedResponse);

}

else{

...

}

}

For the requests I would like to do something similar. I would like to
create or retrieve the session cookie that was previously stored and attach
the corresponding 'cookie' header to the request.

public void processProxyMessage(boolean messageIsRequest,
IInterceptedProxyMessage message){
if(!messageIsRequest){

...

}

else{

Request request = new MyResponse(message.getMessageInfo());

Cookies cookies = Cookies.initFromRequest(request);
String sessionCookie = fetch(request);
cookies.add(sessionCookie);

        byte[] modifiedRequest = request.toBytes();
        message.getMessageInfo().setRequest(modifiedRequest);

}

}

interface Request{
public String getHeader(String name);
}

interface Response{
public setHeader(String name, String value);
}

I hope this makes my usage clearer? But I wouldn't want to restrict it to
just this scenario. It should work the other way around, where the cookie
is removed from the Request or added to the Response.

It could also be possible to use it at the client side (replacing
HttpCookie
https://docs.oracle.com/javase/7/docs/api/java/net/HttpCookie.html).

Ps: How are you dealing with requests that have multiple 'cookie' headers.
You use HttpServletRequest.getHeader() but this method is documented as
follows:

public java.lang.String getHeader(java.lang.String name)
Returns the value of the specified request header as a String. If the
request did not include a header of the specified name, this method returns
null. If there are multiple headers with the same name, this method returns
the first head in the request. The header name is case insensitive. You can
use this method with any request header.
How can you be sure that you are not missing cookie in the other cookie
headers? You only take the first 'cookie' header but I believe multiple
headers are allowed.

I would rather use and make Cookies support the following interface:

interface Request{
public List getHeaders(String name);
}

2015-12-21 17:23 GMT+01:00 Fagner Brack notifications@github.com:

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.


Reply to this email directly or view it on GitHub
#10 (comment)
.

@FagnerMartinsBrack
Copy link
Member

How are you dealing with requests that have multiple 'cookie' headers

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:
It turns out this is invalid, see #11 (comment).
@roelstorms Thanks for the head's up anyway!

@FagnerMartinsBrack
Copy link
Member

Note: Please format you code using java code here , it is easier to read.

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants