rzo1 commented on code in PR #2165:
URL: https://github.com/apache/stormcrawler/pull/2165#discussion_r4029236588


##########
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:
   Switched to `HttpUrl.parse`, test added with `|[]{}^`.



##########
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:
   Right, fixed: the yaml and adoc now list redirect hops and WebSockets as not 
checked, the "okhttp only" header is gone, and the adoc says what happens on 
each side (IOException for OkHttp, aborted request for Playwright).



##########
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:
   Blocked both: the IP check moved to a context route (page route falls back 
to it), so popups are covered, and service workers are blocked while the filter 
is active. Documented as well.



-- 
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]

Reply via email to