dpol1 commented on code in PR #2165:
URL: https://github.com/apache/stormcrawler/pull/2165#discussion_r4026447150
##########
external/playwright/src/main/java/org/apache/stormcrawler/protocol/playwright/HttpProtocol.java:
##########
@@ -188,6 +172,111 @@ public void configure(final Config conf) {
// optional chain of page actions applied after navigate, before
content capture
pageActions = PageActions.fromConf(conf);
+
+ configureIPFilter(conf);
+ }
+
+ /**
+ * Sets up the filtering of the requests made by the browser against
http.filter.ipaddress.*. As
+ * with the OkHttp protocol, fetches through a proxy are not filtered.
+ */
+ void configureIPFilter(final Config conf) {
+ final IPFilterRules rules = new IPFilterRules(conf);
+ if (rules.isEmpty()) {
+ ipFilterRules = null;
+ } else if (StringUtils.isNotBlank(ConfUtils.getString(conf,
"http.proxy"))) {
+ ipFilterRules = null;
+ LOG.info(
+ "http.filter.ipaddress.* do not apply to fetches through a
proxy, the proxy"
+ + " resolves the target host and its own egress
rules decide which"
+ + " addresses are reached");
+ } else {
+ ipFilterRules = rules;
+ }
+ }
+
+ /**
+ * Checks the host of a request made by the browser against the IP filter
rules. The host is
+ * resolved here, separately from the browser, so the address the browser
connects to can
+ * differ, e.g. with short-lived DNS records or a remote browser using
another resolver.
+ *
+ * @return false if any address of the host is rejected or the host cannot
be resolved
+ */
+ boolean isAllowedAddress(final String url) {
+ if (ipFilterRules == null) {
+ return true;
+ }
+ final URI uri;
+ try {
+ uri = new URI(url);
Review Comment:
`new URI` fails on `|`, `[`, `]` (and `{ } ^` in a query), which Chromium
sends as is, and gives no host for a name with `_`: with the default exclude
list those get aborted as forbidden. `HttpUrl.parse` takes them all, and a test
with one of them?
##########
core/src/main/resources/crawler-default.yaml:
##########
@@ -220,6 +220,9 @@ config:
# Fetches through a proxy are not filtered: the proxy resolves and connects
# to the target, so only its address is known here and a proxy on a private
# address is fine. The proxy's own egress rules decide what it may reach.
+ # Applies to the okhttp and playwright protocols. Playwright checks each
+ # request of the browser (page, redirects, subresources) against an address
Review Comment:
redirect hops never go through the handler, only the first url does (the
note above `page.route` says it, and a 302 on an image and on the main page
confirmed it), so this should list redirects with websockets as not checked.
same block still says okhttp only, and the adoc line on failing with an
`IOException` doesn't hold for a blocked subresource.
##########
external/playwright/src/main/java/org/apache/stormcrawler/protocol/playwright/HttpProtocol.java:
##########
@@ -251,6 +340,11 @@ public ProtocolResponse getProtocolOutput(String url,
Metadata md) throws Except
} else if (resourceTypesToSkip.contains(
route.request().resourceType())) {
route.abort();
+ } else if
(!isAllowedAddress(route.request().url())) {
Review Comment:
the handler also misses what a service worker fetches (playwright suggests
`setServiceWorkers(BLOCK)` when routing) and popups, which open unblocked with
no handler at all unless the route sits on the context. block them or document
them, your call
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
