search

oblac / jodd-http

Simple Java HTTP client.
https://http.jodd.org
BSD 2-Clause "Simplified" License
25 stars 9 forks source link

A few thoughts on HttpBrowser #11

Closed neroux closed 2 years ago

neroux commented 2 years ago

Hi @igr,

Just used HttpBrowser for the first time and got a few thoughts on it :)

  1. Generally, I'd probably consider renaming HttpBrowser to HttpSession (or something alike). I realise this will be a breaking change, but I believe that naming would be more appropriate. I understand where the original idea for that name came from, but it still is a bit of a misnomer IMHO, as it is not a browser interface (such Selenium for example) but really rather just a session tracker or cookie jar. What do you think?

  2. Even though https://http.jodd.org/httpbrowser specifically states that HttpBrowser will handle redirects, I am not sure that's really always desirable. There can be cases, where one wants to use it for keeping track of cookies, but still wants to handle redirects manually.

  3. Should the redirect management maybe be unified with HttpRequest? https://github.com/oblac/jodd-http/blob/720a304ee8db25429c69922b21f6587e0a408dc4/src/main/java/jodd/http/HttpBrowser.java#L175-L212

Thanks 🙇🏻

neroux commented 2 years ago

Also, 307 and 308 seem to drop the original request body.

https://github.com/oblac/jodd-http/blob/720a304ee8db25429c69922b21f6587e0a408dc4/src/main/java/jodd/http/HttpBrowser.java#L200-L212

My understanding is, that should not be the case - https://developer.mozilla.org/docs/Web/HTTP/Status/307

neroux commented 2 years ago

Hi @igr, any thoughts on that? Thanks.

igr commented 2 years ago
  1. Agree on the naming! That is correct. I believe it is safe to be renamed; there are not that many users.
igr commented 2 years ago
  1. adding a boolean that will be used for turning off the automatic redirection.
igr commented 2 years ago

Now, regarding the #3 - not sure if I can do it better. The request following feature will re-use the same HTTP request. The session will handle redirects by creating a new request (and storing the original one).

igr commented 2 years ago

I believe I have finished with this task:

neroux commented 2 years ago

Agree on the naming! That is correct. I believe it is safe to be renamed; there are not that many users.

🙇🏻‍♂️

I only voiced the concern because I am always hesitant about such breaking changes :)

adding a boolean that will be used for turning off the automatic redirection.

👍🏻

Now, regarding the https://github.com/oblac/jodd-http/issues/3 - not sure if I can do it better. The request following feature will re-use the same HTTP request. The session will handle redirects by creating a new request (and storing the original one).

That's a fair point.