RFR: 8319332: Security properties files inclusion [v7]

[Francisco Ferrari Bihurriet]

On Fri, 19 Apr 2024 08:16:34 GMT, Martin Balao <mbalao at openjdk.org> wrote:

>> In the compatibility risk description of the CSR:
>>> In line with the efforts to check invalid URLs (see [JDK-8294241](https://bugs.openjdk.org/browse/JDK-8294241): Deprecate URL public constructors), "java.security.properties" file-URL values such as "file:///C:\some\path\extra.properties" or "file:///some/path/extra .properties" need to be converted into their valid counterparts: "file:///C:/some/path/extra.properties" and "file:///some/path/extra%20.properties" respectively.
>> 
>> Is it worth breaking such invalid URLs?
>
>> In the compatibility risk description of the CSR:
>> 
>> > In line with the efforts to check invalid URLs (see [JDK-8294241](https://bugs.openjdk.org/browse/JDK-8294241): Deprecate URL public constructors), "java.security.properties" file-URL values such as "file:///C:\some\path\extra.properties" or "file:///some/path/extra .properties" need to be converted into their valid counterparts: "file:///C:/some/path/extra.properties" and "file:///some/path/extra%20.properties" respectively.
>> 
>> Is it worth breaking such invalid URLs?
> 
> I think that it is worth introducing this breaking change now, under a release note. Adjustments may be required to some Java launchers but the fix in itself should be straightforward. Those malformed URLs should have never been allowed and, with the deprecation and possible removal of the constructor that allowed them, it will be something to do at some point anyways. If we continue to support them, we would need to introduce code to handle includes different than `java.security.properties` or accept that the malformed URLs will spread to newly included files —and breaking changes will be worse in the future—.

In reply to @martinuy's [response](https://github.com/openjdk/jdk/pull/16483#issuecomment-2066004531) to @wangweij:
> [...]
>
> Adding this to the CSR sounds like a good idea. I'm certain that @franferrax will agree.

I agree, I can add this next Monday.

In reply to @martinuy's [response](https://github.com/openjdk/jdk/pull/16483#issuecomment-2066069129) to @wangweij:
> [...]
> > 
> > Is it worth breaking such invalid URLs?
> 
> I think that it is worth introducing this breaking change now, under a release note. Adjustments may be required to some Java launchers but the fix in itself should be straightforward. Those malformed URLs should have never been allowed and, with the deprecation and possible removal of the constructor that allowed them, it will be something to do at some point anyways. If we continue to support them, we would need to introduce code to handle includes different than `java.security.properties` or accept that the malformed URLs will spread to newly included files —and breaking changes will be worse in the future—.

Let me also note that this has been already discussed in @AlanBateman's [initial review](https://github.com/openjdk/jdk/pull/16483#pullrequestreview-1711145372), where we decided to go with this breaking change to remove the dependency on `FileURLConnection` (which we had introduced for a fully backwards-compatible parsing).

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2066529564
PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2066531414