guardian / guardian/prout

Support checkpoint urls that require cookies

Open
#111 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.