Support checkpoint urls that require cookies
- Dominant language
- Scala
- Stars
- 147
- Forks
- 15
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 2
Description
Some Guardian endpoints like https://support.theguardian.com/uk/contribute now require an Okta cookie, and will redirect endlessly if the cookie can't be dropped (see eg https://github.com/guardian/support-frontend/pull/5152#issuecomment-1653900649):
```
$ curl -I https://support.theguardian.com/uk/contribute
HTTP/2 303
location: /oauth/authorize
```
Giving us an error like this:
```
Could not read from this url, got java.net.ProtocolException: Too many follow-up requests: 21
```
We wouldn't want Prout to maintain cookies across separate checkpoint snapshots (each snapshot should be isolated), but when taking a single snapshot, which might redirect (eg in order to _acquire_ cookies), it could be that we _do_ want to maintain cookies.
Prout currently uses OkHttp as the HTTP client used to take checkpoint snapshots:
https://github.com/guardian/prout/blob/705f02af7e00d59be83a7f1dcaceb2146ce0dd5b/app/lib/CheckpointSnapshot.scala#L40
## Switch HTTP client to `java.net.http.HttpClient` ?
Documentation on how to get OkHttp to store cookies between requests (with a `CookieJar`) is [limited](https://square.github.io/okhttp/4.x/okhttp/okhttp3/-cookie-jar/), but Java 9 introduced an alternative HTTP client, [`java.net.http.HttpClient`](https://docs.oracle.com/en/java/javase/11/docs/api/java.net.http/java/net/http/HttpClient.html):
* https://www.baeldung.com/java-9-http-client
* https://openjdk.org/groups/net/httpclient/intro.html
...which seems to supply all of Prout's requirements in a HTTP client, including this new one for cookies:
* **Cookies** : [`java.net.CookieManager`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/CookieManager.html) seems to offer a suitable level of cookie control
* **Redirects** : [`HttpClient.Redirect`](https://docs.oracle.com/en/java/javase/11/docs/api/java.net.http/java/net/http/HttpClient.Redirect.html) lets us enable redirects - and by default, there is a [redirect limit of 5](https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/19fb8f93c59dfd791f62d41f332db9e306bc1422/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java#L85-L87), which should be fine.
* **Disabling security** : https://github.com/guardian/prout/pull/19 can be supported with https://stackoverflow.com/q/52988677/438886
* **Async** : See [`google-search-indexing-observatory`](https://github.com/guardian/google-search-indexing-observatory/blob/8bf9b1c9d6f98af85e17ffec572f016d4afe1913/src/main/scala/ophan/google/indexing/observatory/SitemapDownloader.scala#L28-L42) for an example of `java.net.http.HttpClient` being used asynchronously with Scala code & `Future`s.
Of course, switching to the JDK HTTP client would also mean we'd get to drop a dependency, which would be nice!
Contributor guide
Research direction
Start with app/lib/CheckpointSnapshot.scala around line 40, then compare the Java HTTP client documentation and the asynchronous Scala example in SitemapDownloader.scala. Verify how cookies, redirects, disabled security, and snapshot isolation should work before choosing the client. Done means a single snapshot can acquire and reuse cookies without carrying them into another snapshot, while avoiding the redirect loop.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, scala
- Domain
- backend, networking
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100