Skip to content

Validate HTTP response codes across the extractor #1322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
+81 −14
Open

Validate HTTP response codes across the extractor #1322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
58094d8
[SoundCloud] Validate http response code in SoundcloudParsingHelper
absurdlylongusername Jun 8, 2025
ae4e8fc
Move validateResponseCode to Response
absurdlylongusername Oct 5, 2025
e4c5117
Fix checkstyle off
absurdlylongusername Oct 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions extractor/src/main/java/org/schabi/newpipe/extractor/downloader/Response.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;

import org.schabi.newpipe.extractor.exceptions.HttpResponseException;

/**
* A Data class used to hold the results from requests made by the Downloader implementation.
*/
Expand All @@ -30,6 +33,29 @@ public Response(final int responseCode,
this.latestUrl = latestUrl;
}

// CHECKSTYLE:OFF
/**
* Validates the response codes for the given {@link Response}, and throws
* a {@link HttpResponseException} if the code is invalid
* @param response The response to validate
* @param validResponseCodes Expected valid response codes
* @throws HttpResponseException Thrown when the response code is not in {@code validResponseCodes},
* or when {@code validResponseCodes} is empty and the code is a 4xx or 5xx error.
*/
// CHECKSTYLE:ON
public static void validateResponseCode(final Response response,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not inline this directly within the non-static Response::validateResponseCode? This is basically a method on the Response class since it takes Response as the first argument, so you might as well make it non-static.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to allow for response.validateResponseCode and validateResponseCode(response) depending on the format of the call site.

But in hindsight, I now recall that in C# (my main language) there is EnsureSuccessStatusCode which can be used on the response itself, rather than a separate method.

So yeah I think you're right, validation can just be done inline like how you have mentioned in your other comment

final int... validResponseCodes)
throws HttpResponseException {
final int code = response.responseCode();
final var throwError = (validResponseCodes == null || validResponseCodes.length == 0)
? code >= 400 && code <= 599
: Arrays.stream(validResponseCodes).noneMatch(c -> c == code);

if (throwError) {
throw new HttpResponseException(response);
}
}

public int responseCode() {
return responseCode;
}
Expand Down Expand Up @@ -80,4 +106,22 @@ public String getHeader(final String name) {

return null;
}

// CHECKSTYLE:OFF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why checkstyle off?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, checkstyle was complaining about line being too long in the comment below, so I had to disable it

Copy link
Member

@Stypox Stypox Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really important, but why don't you make the lines shorter then? Here you don't have long URLs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

/**
* Helper function simply to make it easier to validate response code inline
* before getting the code/body/latestUrl/etc.
* Validates the response codes for the given {@link Response}, and throws a {@link HttpResponseException} if the code is invalid
* @see Response#validateResponseCode(Response, int...)
* @param validResponseCodes Expected valid response codes
* @return {@link this} response
* @throws HttpResponseException Thrown when the response code is not in {@code validResponseCodes},
* or when {@code validResponseCodes} is empty and the code is a 4xx or 5xx error.
*/
// CHECKSTYLE:ON
public Response validateResponseCode(final int... validResponseCodes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to support the needs of other services (like youtube), I'd add the following methods:

public Response throwIfResponseCode(int errorCode, final Function<Response, HttpResponseException> errorSupplier);
public Response throwIfResponseCodeInRange(final int errorCodeMin, final int errorCodeMax, final Function<Response, HttpResponseException> errorSupplier);

I'd also rename this method to throwIfInvalidResponseCode()

Then e.g. for YouTube we'd be able to do:

response
    // "https://www.youtube.com" is the URL to solve the captcha
    .throwIfResponseCode(429, r -> new ReCaptchaException(r, "https://www.youtube.com"))
    .throwIfInvalidResponseCode()

While in PeerTube

response
    .throwIfResponseCode(429, r -> new TooManyRequestsException(r))
    .throwIfInvalidResponseCode()

And we'd remove the ReCaptchaException checks in the Downloader. Also we'd have ReCaptchaException and TooManyRequestsException extend HttpResponseException.

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this.

However, I think throwIfInvalidResponseCode should be called ensureValidResponseCode or ensureSuccessResponseCode if it's going to be the same as validateResponseCode which takes in a list of valid response codes. Consider these:

  • throwIfInvalidResponseCode(200, 201)
  • ensureValidResponseCode(200, 201)

Reading the second, it is easier to understand that 200 and 201 are the valid response codes, but one could presume with the first that 200 and 201 are invalid response codes and it should throw if one of those are returned.

Thoughts?

Copy link
Member

@Stypox Stypox Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureValidResponseCode is a better name indeed

throws HttpResponseException {
validateResponseCode(this, validResponseCodes);
return this;
}
}
14 changes: 14 additions & 0 deletions extractor/src/main/java/org/schabi/newpipe/extractor/exceptions/HttpResponseException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.schabi.newpipe.extractor.exceptions;

import org.schabi.newpipe.extractor.downloader.Response;

public class HttpResponseException extends ExtractionException {
public HttpResponseException(final Response response) {
this("Error in HTTP Response for " + response.latestUrl() + "\n\t"
+ response.responseCode() + " - " + response.responseMessage());
}

public HttpResponseException(final String message) {
super(message);
}
}
37 changes: 23 additions & 14 deletions ...c/main/java/org/schabi/newpipe/extractor/services/soundcloud/SoundcloudParsingHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
import static org.schabi.newpipe.extractor.ServiceList.SoundCloud;
import static org.schabi.newpipe.extractor.utils.Utils.isNullOrEmpty;
import static org.schabi.newpipe.extractor.utils.Utils.replaceHttpWithHttps;
import static org.schabi.newpipe.extractor.downloader.Response.validateResponseCode;

import com.grack.nanojson.JsonArray;
import com.grack.nanojson.JsonObject;
import com.grack.nanojson.JsonParser;
import com.grack.nanojson.JsonParserException;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;
import org.jsoup.select.Elements;
import org.schabi.newpipe.extractor.MultiInfoItemsCollector;
import org.schabi.newpipe.extractor.Image;
Expand All @@ -21,6 +21,7 @@
import org.schabi.newpipe.extractor.downloader.Downloader;
import org.schabi.newpipe.extractor.downloader.Response;
import org.schabi.newpipe.extractor.exceptions.ExtractionException;
import org.schabi.newpipe.extractor.exceptions.HttpResponseException;
import org.schabi.newpipe.extractor.exceptions.ParsingException;
import org.schabi.newpipe.extractor.exceptions.ReCaptchaException;
import org.schabi.newpipe.extractor.services.soundcloud.extractors.SoundcloudChannelInfoItemExtractor;
Expand Down Expand Up @@ -104,8 +105,8 @@ public static synchronized String clientId() throws ExtractionException, IOExcep

final Downloader dl = NewPipe.getDownloader();

final Response download = dl.get("https://soundcloud.com");
final String responseBody = download.responseBody();
final Response downloadResponse = dl.get("https://soundcloud.com").validateResponseCode();
final String responseBody = downloadResponse.responseBody();
final String clientIdPattern = ",client_id:\"(.*?)\"";

final Document doc = Jsoup.parse(responseBody);
Expand All @@ -116,11 +117,12 @@ public static synchronized String clientId() throws ExtractionException, IOExcep

final var headers = Map.of("Range", List.of("bytes=0-50000"));

for (final Element element : possibleScripts) {
for (final var element : possibleScripts) {
final String srcUrl = element.attr("src");
if (!isNullOrEmpty(srcUrl)) {
try {
clientId = Parser.matchGroup1(clientIdPattern, dl.get(srcUrl, headers)
.validateResponseCode()
.responseBody());
return clientId;
} catch (final RegexException ignored) {
Expand Down Expand Up @@ -148,11 +150,13 @@ public static OffsetDateTime parseDateFrom(final String textualUploadDate)
}
}

// CHECKSTYLE:OFF
/**
* Call the endpoint "/resolve" of the API.<p>
* Call the endpoint "/resolve" of the API.
* <p>
* See https://developers.soundcloud.com/docs/api/reference#resolve
* See https://web.archive.org/web/20170804051146/https://developers.soundcloud.com/docs/api/reference#resolve
*/
// CHECKSTYLE:ON
public static JsonObject resolveFor(@Nonnull final Downloader downloader, final String url)
throws IOException, ExtractionException {
final String apiUrl = SOUNDCLOUD_API_V2_URL + "resolve"
Expand All @@ -175,12 +179,13 @@ public static JsonObject resolveFor(@Nonnull final Downloader downloader, final
* @return the url resolved
*/
public static String resolveUrlWithEmbedPlayer(final String apiUrl) throws IOException,
ReCaptchaException {
ReCaptchaException, HttpResponseException {

final String response = NewPipe.getDownloader().get("https://w.soundcloud.com/player/?url="
+ Utils.encodeUrlUtf8(apiUrl), SoundCloud.getLocalization()).responseBody();

return Jsoup.parse(response).select("link[rel=\"canonical\"]").first()
final var response = NewPipe.getDownloader().get("https://w.soundcloud.com/player/?url="
+ Utils.encodeUrlUtf8(apiUrl), SoundCloud.getLocalization());
validateResponseCode(response);
final var responseBody = response.responseBody();
return Jsoup.parse(responseBody).select("link[rel=\"canonical\"]").first()
.attr("abs:href");
}

Expand All @@ -189,6 +194,7 @@ public static String resolveUrlWithEmbedPlayer(final String apiUrl) throws IOExc
*
* @return the resolved id
*/
// TODO: what makes this method different from the others? Don' they all return the same?
public static String resolveIdWithWidgetApi(final String urlString) throws IOException,
ParsingException {
String fixedUrl = urlString;
Expand Down Expand Up @@ -224,9 +230,12 @@ public static String resolveIdWithWidgetApi(final String urlString) throws IOExc
final String widgetUrl = "https://api-widget.soundcloud.com/resolve?url="
+ Utils.encodeUrlUtf8(url.toString())
+ "&format=json&client_id=" + SoundcloudParsingHelper.clientId();
final String response = NewPipe.getDownloader().get(widgetUrl,
SoundCloud.getLocalization()).responseBody();
final JsonObject o = JsonParser.object().from(response);

final var response = NewPipe.getDownloader().get(widgetUrl,
SoundCloud.getLocalization());

final var responseBody = response.validateResponseCode().responseBody();
final JsonObject o = JsonParser.object().from(responseBody);
return String.valueOf(JsonUtils.getValue(o, "id"));
} catch (final JsonParserException e) {
throw new ParsingException("Could not parse JSON response", e);
Expand Down