[🐛 Bug]: NetworkInterceptor overwrites failed requests with 200 response

[joebandenburg]

What happened?

I've been using NetworkInterceptor successfully with Chrome to add headers to requests. However, I've run into an edge case where NetworkInterceptor does the wrong thing.

If the request fails by not returning a response (e.g. there's a timeout or the hostname does not exist), Selenium replaces the response with a empty 200 response and returns that to Chrome. This causes Chrome to load an empty page rather than failing.

As you can see from the logs below, the second Fetch.requestPaused message from Chrome contains "responseErrorReason":"NameNotResolved". After this Selenium sends an inappropriate Fetch.getResponseBody, which fails. Selenium constructs an empty 200 response and passes that back to the user's filter. Whatever that filter return is then sent via a Fetch.fulfillRequest call. As far as I can tell, there is no way for the user's filter to distinguish one of these fake 200 responses for a real 200 response (other than the fact that the response is otherwise empty).

I'm not entirely sure how Selenium should handle this error but sending a Fetch.continueRequest seems to make sense and makes the behaviour consistent to how Selenium behaves if the NetworkInterceptor has not been used. i.e. the web driver will throw a WebDriverException from driver.get.

The faulty logic seems to be in v123Network#createSeMessages. It checks to see if responseErrorReason is set but otherwise ignores the value, continuing on to try to fetch the response body. When that fails, it assumes the reason for such a failure is due to a redirect response, at which point an empty 200 response is constructed.

How can we reproduce the issue?

WebDriver driver = ...
Filter filter = next -> next;
try (var ignored = new NetworkInterceptor(driver, filter)) {
  // This should raise an exception, but doesn't!
  driver.get("https://fakesite.pro/en/index.html");
  // The web browser has loaded an empty page, even though fakesite.pro doesn't exist.
}

Relevant log output

2024-04-03T12:53:33.759-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : <- {"method":"Fetch.requestPaused","params":{"requestId":"interception-job-1.0","request":{"url":"https://fakesite.pro/en/index.html","method":"GET","headers":{"Accept":"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7","Accept-Language":"en-GB,en-US;q=0.9,en;q=0.8","Upgrade-Insecure-Requests":"1","User-Agent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Safari/537.36"},"initialPriority":"VeryHigh","referrerPolicy":"strict-origin-when-cross-origin"},"frameId":"618FD10968BEC9788FF3163BFDE85FAD","resourceType":"Document"},"sessionId":"F9C647A6D0F886EAE174C7CFD101EF46"}
2024-04-03T12:53:33.760-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : Method Fetch.requestPaused called with 2 callbacks available
2024-04-03T12:53:33.760-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : Matching Fetch.requestPaused with Fetch.requestPaused
2024-04-03T12:53:33.762-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : Calling callback for Fetch.requestPaused using org.openqa.selenium.devtools.idealized.Network$$Lambda/0x000000050184e7b8@34b31eb3 being passed org.openqa.selenium.devtools.v123.fetch.model.RequestPaused@14ca5f7b
2024-04-03T12:53:33.763-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : -> {
  "id": 7,
  "sessionId": "F9C647A6D0F886EAE174C7CFD101EF46",
  "method": "Fetch.continueRequest",
  "params": {
    "postData": "",
    "requestId": "interception-job-1.0",
    "method": "GET",
    "headers": [
      {
        "name": "Accept",
        "value": "text\u002fhtml,application\u002fxhtml+xml,application\u002fxml;q=0.9,image\u002favif,image\u002fwebp,image\u002fapng,*\u002f*;q=0.8,application\u002fsigned-exchange;v=b3;q=0.7"
      },
      {
        "name": "Upgrade-Insecure-Requests",
        "value": "1"
      },
      {
        "name": "User-Agent",
        "value": "Mozilla\u002f5.0 (X11; Linux x86_64) AppleWebKit\u002f537.36 (KHTML, like Gecko) Chrome\u002f123.0.0.0 Safari\u002f537.36"
      },
      {
        "name": "Accept-Language",
        "value": "en-GB,en-US;q=0.9,en;q=0.8"
      }
    ],
    "url": "https:\u002f\u002ffakesite.pro\u002fen\u002findex.html"
  }
}
2024-04-03T12:53:33.766-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : <- {"id":7,"result":{},"sessionId":"F9C647A6D0F886EAE174C7CFD101EF46"}
2024-04-03T12:53:33.777-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : <- {"method":"Fetch.requestPaused","params":{"requestId":"interception-job-1.0","request":{"url":"https://fakesite.pro/en/index.html","method":"GET","headers":{"Accept":"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7","Accept-Language":"en-GB,en-US;q=0.9,en;q=0.8","Upgrade-Insecure-Requests":"1","User-Agent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Safari/537.36"},"initialPriority":"VeryHigh","referrerPolicy":"strict-origin-when-cross-origin"},"frameId":"618FD10968BEC9788FF3163BFDE85FAD","resourceType":"Document","responseErrorReason":"NameNotResolved"},"sessionId":"F9C647A6D0F886EAE174C7CFD101EF46"}
2024-04-03T12:53:33.778-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : Method Fetch.requestPaused called with 2 callbacks available
2024-04-03T12:53:33.778-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : Matching Fetch.requestPaused with Fetch.requestPaused
2024-04-03T12:53:33.778-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : Calling callback for Fetch.requestPaused using org.openqa.selenium.devtools.idealized.Network$$Lambda/0x000000050184e7b8@34b31eb3 being passed org.openqa.selenium.devtools.v123.fetch.model.RequestPaused@744853f6
2024-04-03T12:53:33.779-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : -> {
  "id": 8,
  "sessionId": "F9C647A6D0F886EAE174C7CFD101EF46",
  "method": "Fetch.getResponseBody",
  "params": {
    "requestId": "interception-job-1.0"
  }
}
2024-04-03T12:53:33.781-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : <- {"id":8,"error":{"code":-32000,"message":"Can only get response body on requests captured after headers received."},"sessionId":"F9C647A6D0F886EAE174C7CFD101EF46"}
2024-04-03T12:53:33.782-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : Matching Fetch.requestPaused with Fetch.authRequired
2024-04-03T12:53:33.783-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : -> {
  "id": 9,
  "sessionId": "F9C647A6D0F886EAE174C7CFD101EF46",
  "method": "Fetch.fulfillRequest",
  "params": {
    "responseHeaders": [
    ],
    "body": "",
    "requestId": "interception-job-1.0",
    "responseCode": 200
  }
}
2024-04-03T12:53:33.785-04:00  INFO 33243 --- [ CDP Connection] o.openqa.selenium.devtools.Connection    : <- {"id":9,"result":{},"sessionId":"F9C647A6D0F886EAE174C7CFD101EF46"}

Operating System

macOS Sonoma 14.4.1

Selenium version

Java 4.19.1

What are the browser(s) and version(s) where you see this issue?

Chrome 123.0

What are the browser driver(s) and version(s) where you see this issue?

ChromeDriver 123.0

Are you using Selenium Grid?

4.19.1

[github-actions]

@joebandenburg, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[joebandenburg]

A workaround, for those that never want to modify the response in their filter, is to always return PROCEED_WITH_REQUEST instead of the response returned by the chained filter.

So instead of:

Filter f = next -> req -> {
  // Alter the req
  return next.execute(req);
}

Do this:

Filter f = next -> req -> {
  // Alter the req
  next.execute(req);
  return PROCEED_WITH_REQUEST;
}

[pujagani]

Thank you for the details. I am able to reproduce the problem exactly how it is described. Trying to find a solution for the same now.

[pujagani]

The above PR has an approach where, if it is a known error to ChromeDevTools, then the response contains the error reason, I have added a check for this error reason and if it is present, then we continue the request without any modification, which leads to the normal expected behaviour i.e. throw an error.
Does that make sense? Is there a use case that I overlooked here for which this fix might become a bug later?

[joebandenburg]

The fix looks like a nice way to solve the bug. Thanks!

I think the fix is good to merge as it, but there is a possible extension that I think is worth considering. I can imagine a hypothetical scenario where the user may want to intercept the failing request and generate a successful response (essentially the opposite of what I want to do here). One way to do that without radically changing the API is to throw an exception back up the filter chain and if that exception makes it to the top of the chain then call continueWithoutModification. This gives the user to catch this exception in their filter and choose to return a HttpResponse instead, if they want. What do you think of this idea?

I'm trying to mock this up in a PR but I'm struggling with Bazel and debugging in IntelliJ.

[joebandenburg]

What do you think of this idea?

I've created a draft PR to demonstrate the idea above.

[pujagani]

Your idea looks good to me. I will merge the changes from my PR and then merge your PR suggestion (can you open it up for review please). Thank you for your insights and contribution, I think both solutions will address all use-cases.

[github-actions]

This issue has been automatically locked since there has not been any recent activity since it was closed. Please open a new issue for related bugs.