[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8254049: Update WebView to public suffix list 2020-04-24
From:       Kevin Rushforth <kcr () openjdk ! java ! net>
Date:       2020-10-27 0:45:18
Message-ID: poOT1MVU4hMektbexfTMl2XbLds0JAqHDcqL2QbJg-Y=.80467af3-8d93-4faf-b02c-815725baac20 () github ! com
[Download RAW message or body]

On Mon, 19 Oct 2020 04:29:13 GMT, Arun Joseph <ajoseph@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


[prev in list] [next in list] [prev in thread] [next in thread]