Break up HTTPUtilities into several classes per SRP

[GoogleCodeExporter]

(Kevin)

Some possibilities... Make a new _package_ called org.owasp.esapi.httputilities
and then either divide things into separate classes in that package based on
what aspect of HTTP they are addressing (e.g., RequestUtils, ResponseUtils,
CookieUtils, etc.) or what their primary intended purpose / function is, such
as FileUploadUtils, ParameterManipulationUtils, HeaderManipulationUtils, etc.
Either way, you are bound to have a few odd cases, but that should allow you
to get things down to a manageable size in terms of the # of methods in any
given interface.

Original issue reported on code.google.com by manico.james@gmail.com on 4 Nov 2010 at 7:15

[GoogleCodeExporter]

(Chris)

First off, I think the HttpUtilities interface really just needs to go away.
I really don't see the benefit in creating this as an interface and having
to configure it - these are utilities that should be done the same way
across the board for any container that I can think of.

Second, I absolutely agree, this thing is unwieldy and hard to maintain. I
vote for splitting it up into the following:

HttpRequestHelper
HttpResponseHelper
HttpSessionHelper

I would also like to see the following done:

New class SecureCookieManager where all cookie related methods go -

New subclasses of javax.servlet.http.Cookie -> SecureCookie (forces
http-only and secure flags) and EncryptedCookie (encapsulates the encryption
and decryption of cookie values behind the scenes)

My main concern here is a change to a *core* interface so late in the game.
I would love to see this in 2.0GA but I find myself a little hesitant to do
so at this point because we have already called out that there will be no
further interface level changes before 2.0GA

That being said - perhaps it would behoove us to create these now, then
deprecate the HttpUtilties interface and DefaultHttpUtilities class and just
delegate to the correct Helper and new code.

Also - I think it would be worthwhile to move the current ThreadLocals that
are stored on the HttpUtilities object up to the Locator level (since I
think most people get to them at that level anyhow)

ESAPI.setCurrent()
ESAPI.currentRequest()
ESAPI.currentResponse()

Original comment by manico.james@gmail.com on 4 Nov 2010 at 7:17

[GoogleCodeExporter]

(Sebastian)

Remember that it is always a good idea to look at
the methods when you want to break up a class.
If there are method names and they have a noun (the object in the
grammatical sense) in common, it is often the noun that wants to be
freed as a stand alone class.

E.g. There are several methods with the noun 'cookie' in HTTPUtilities:

public void addCookie(HttpServletResponse response, Cookie cookie)
public Map<String,String> decryptStateFromCookie(
    HttpServletRequest request)
public void encryptStateInCookie(HttpServletResponse response,
    Map<String,String> cleartext)

You could fix the naming conflict with the
javax cookie class if you take the plural "Cookies".
This is justified as the operations really act on
the collection of cookies in the request/response.

So the extracted interface could look as follows:

interface Cookies {
public void add(HttpServletResponse response, Cookie cookie);
public Map<String,String> decryptStateFrom(
    HttpServletRequest request);
public void encryptStateIn(HttpServletResponse response,
    Map<String,String> cleartext);
}

Here's what I found in HTTPUtilities but I am getting to the limits
of my English here so I'm sure you'll find better names
and better abstractions:

* HTTPRequestLogger
* CSRFTokenManager // It would be nice if Token verification was
                   // also done herein...
* SessionCloner
* FileUploadExtractor
* HiddenFieldEncryptor
* HTTPHeaders

However, I would keep the external interface as it is for this release
to make the transition easier for people already using ESAPI 1.4
and the 2.0 RCs.

Original comment by manico.james@gmail.com on 6 Nov 2010 at 9:28

[GoogleCodeExporter]

Original comment by manico.james@gmail.com on 29 May 2012 at 3:25