rzo1 commented on code in PR #2124:
URL: https://github.com/apache/stormcrawler/pull/2124#discussion_r3944167440
##########
core/src/main/resources/crawler-default.yaml:
##########
@@ -215,10 +215,16 @@ config:
robots.cache.spec: "maximumSize=10000,expireAfterWrite=6h"
robots.error.cache.spec: "maximumSize=10000,expireAfterWrite=1h"
- protocols: "http,https,file"
+ # The file scheme must be enabled deliberately: a fetched page can put a
Review Comment:
This is a breaking change for anyone crawling local files: they now have to
set both `protocols` and `file.protocol.root`. It needs to be in the release
notes.
Six PRs in this batch change shipped defaults (#2119, #2123, #2124, #2125,
#2126, #2127). Can one of them carry a combined upgrade note?
##########
core/src/main/java/org/apache/stormcrawler/protocol/file/FileResponse.java:
##########
@@ -61,6 +61,23 @@ public FileResponse(String u, Metadata md, FileProtocol
fileProtocol) throws IOE
File file = new File(URLDecoder.decode(path,
fileProtocol.getEncoding()));
+ /*
+ * A URL decides which path the worker opens: without a configured root
+ * nothing is served, with one the resolved path must stay below it.
+ */
+ File root = fileProtocol.getRoot();
+ if (root == null) {
+ LOG.warn("Refusing to read {} because {} is not configured", url,
FileProtocol.ROOT_KEY);
Review Comment:
The canonical check and the read are separate operations, so a symlink
swapped in between still wins. That is acceptable now that `file:` is opt-in
and root-confined, but please add a line saying so, otherwise the next reader
redoes the analysis.
--
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]
