bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.67k stars 6.12k forks source link

GlideUrl fix for urls with IPV6 addresses #5444

Open bennettpeter opened 2 months ago

bennettpeter commented 2 months ago

IPV6 URLs are of the form http://[2607:f8b0:4006:823::200e]/ but the code was removing the square brackets from the URL, resulting in an invalid URL and a MalformedURLException.

Description

Cater for IPV6 URLS of the form http://[2607:f8b0:4006:823::200e]/

Motivation and Context

Glide throws exceptions and fails if using IPV6 IP Addresses

kanelbulle commented 1 month ago

Could you please add a test for this?

bennettpeter commented 1 month ago

I assume that you are referring to "Test cases for issues" described in https://bumptech.github.io/glide/tut/failing-test-cases.html . I will work on that.

bennettpeter commented 1 month ago

I am having a problem creating a test case as documented in https://bumptech.github.io/glide/tut/failing-test-cases.html . I followed the directions and created instrumentation/src/androidTest/java/com/bumptech/glide/Issue5444Test.java . Trying to run it by using "Run Issue5444test" results in the following errors:

The option setting 'android.experimental.testOptions.emulatorSnapshots.maxSnapshotsForTestFailures=0' is experimental.
Please correct the above warnings first.

The same happens if I try to run any of the tests that already exist in the directory instrumentation/src/androidTest/java/com/bumptech/glide

I tried updating annotation/compiler/proguard.pro by adding -ignorewarnings at the end. This causes the following errors when running my test or existing tests in the directory::

No SupportedSourceVersion annotation found on com.bumptech.glide.annotation.compiler.GlideAnnotationProcessor, returning RELEASE_6.
Supported source version 'RELEASE_6' from annotation processor 'org.gradle.api.internal.tasks.compile.processing.TimeTrackingProcessor' less than -source '11'
No SupportedAnnotationTypes annotation found on com.bumptech.glide.annotation.compiler.GlideAnnotationProcessor, returning an empty set.
warnings found and -Werror specified
kanelbulle commented 1 month ago

I assume that you are referring to "Test cases for issues" described in https://bumptech.github.io/glide/tut/failing-test-cases.html . I will work on that.

There is an existing test file for this class already, GlideUrlTest.java, can you add to that?

bennettpeter commented 1 month ago

I have added a test case to branch issue5444test. The test case fails because of the reported bug and succeeds if you merge this pull request IPV6-fix. The test uses URL http://[2600:1f13:37c:1400:ba21:7165:5fc7:736e]/ which is a valid internet URL.

kanelbulle commented 1 month ago

Thank you! Could you please combine the changes into this PR so they can get tested & merged at the same time?

bennettpeter commented 1 month ago

As requested, I have rebased and merged issue issue5444test into pull request IPV6-fix. The test runs successfully.

kanelbulle commented 2 weeks ago

thank you!