Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GlideUrl fix for urls with IPV6 addresses #5444

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bennettpeter
Copy link

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

@bennettpeter bennettpeter marked this pull request as ready for review September 5, 2024 17:50
@kanelbulle
Copy link
Collaborator

Could you please add a test for this?

@bennettpeter
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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

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.
@bennettpeter
Copy link
Author

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

@kanelbulle
Copy link
Collaborator

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants