On Sun, 6 Sep 2020 11:21:58 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review and a sponsor for a fix for the issue reported at >> https://bugs.openjdk.java.net/browse/JDK-8252767? >> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` >> is throwing a `IllegalAccessError` instead >> of a `IllegalStateException`. The commit here fixes that and includes a test >> which reproduces the issue and verifies >> the fix. Would a CSR be needed for this change? > > Jaikiran Pai has refreshed the contents of this pull request, and previous > commits have been removed. The incremental > views will show differences compared to the previous content of the PR. The > pull request contains one new commit since > the last revision: > 8252767: URLConnection.setRequestProperty throws IllegalAccessError > Summary: Throw an IllegalStateException from > sun.net.www.URLConnection#setRequestProperty when already connected > Reviewed-By: Overall, this looks good. I added a few minor test related comments. test/jdk/java/net/URLConnection/RequestProperties.java line 90: > 88: * an {@link IllegalStateException} when already connected > 89: * > 90: * @throws Exception This @throws seems superfluous. Please remove. test/jdk/java/net/URLConnection/RequestProperties.java line 97: > 95: conn.connect(); > 96: try { > 97: conn.setRequestProperty("foo", "bar"); While here, and not directly related to this change, it would be good to add coverage for the other xxxRequestPropertXXX methods - to ensure that they also throw the correction exception when connected. ------------- PR: https://git.openjdk.java.net/jdk/pull/26
