On Mon, 19 Oct 2020 04:29:13 GMT, Arun Joseph <ajos...@openjdk.org> wrote:
> We should use the public_suffix_list.dat file in the JDK instead. Reading the > public_suffix_list.dat file is modified to be similar to > [DomainName.java](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/util/DomainName.java). > If the file is not present, `isPublicSuffix()` returns `false`, which is > similar to how WebKit ignores the public suffix check when it is disabled. > > Test: Run PublicSuffixesTest.java I haven't done an exhaustive review yet, but it generally seems to work. I left a few questions below. modules/javafx.web/src/main/java/com/sun/webkit/network/PublicSuffixes.java line 69: > 67: * The mapping from domain names to public suffix list rules. > 68: */ > 69: private static final Map<String, Rules> rulesCache = new > ConcurrentHashMap<>(); I'm not sure that this needs to be concurrent, especially if you initialize it in a static init block (see my comment below). modules/javafx.web/src/main/java/com/sun/webkit/network/PublicSuffixes.java line 132: > 130: return null; > 131: } > 132: return rulesCache.computeIfAbsent(tld, k -> > createRules(tld)); If the public_suffixes file cannot be loaded (for example, if it is removed from the JDK), then this causes a warning to be logged on every call to this (basically on every load of any web site). It might be better to call `createRules` from a static init block. modules/javafx.web/src/main/java/com/sun/webkit/network/PublicSuffixes.java line 89: > 87: > 88: Rules rules = Rules.getRules(domain); > 89: return rules == null ? false : rules.match(domain); This will return `false` if there is no `public_suffix_list.dat` file or if there is an error parsing it. Can you confirm that this is the desired fallback behavior? ------------- PR: https://git.openjdk.java.net/jfx/pull/324
