Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion jablib/src/main/java/org/jabref/logic/util/URLUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,25 @@ public static boolean isURL(String url) {
* @throws MalformedURLException if the URL is malformed and cannot be converted to a {@link URL}.
*/
public static URL create(String url) throws MalformedURLException {
return createUri(url).toURL();
try {
if (url == null || url.trim().isEmpty()) {
throw new MalformedURLException("Provided URL is null or empty.");
}
URI parsedUri = new URI(url.trim());
if (!parsedUri.isAbsolute()) {
throw new MalformedURLException("URI is not absolute: " + url);
}
if (parsedUri.getScheme() == null || parsedUri.getHost() == null) {
throw new MalformedURLException("URI must include both scheme and host: " + url);
}
return parsedUri.toURL();
} catch (URISyntaxException e) {
throw new MalformedURLException("Invalid URI syntax: " + url + " | Error: " + e.getMessage());
} catch (IllegalArgumentException e) {
throw new MalformedURLException("Illegal argument in URI construction: " + url + " | Error: " + e.getMessage());
} catch (NullPointerException e) {
throw new MalformedURLException("Null value encountered during URI parsing: " + url);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed by checking for url in the beginning of the method. can now be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes , I removed this lines of code.

}

/**
Expand Down
67 changes: 64 additions & 3 deletions jablib/src/test/java/org/jabref/logic/net/URLUtilTest.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package org.jabref.logic.net;

import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;

import org.jabref.logic.util.URLUtil;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.*;

class URLUtilTest {

Expand Down Expand Up @@ -87,4 +88,64 @@ void createUriShouldHandlePipeCharacter() {
URI uri = URLUtil.createUri(input);
assertEquals("http://example.com/test%7Cfile", uri.toString());
}

// valid absolute URL
@Test
public void testValidUrl() {
String input = "http://example.com";

try {
URL result = URLUtil.create(input);
assertNotNull(result, "URL should not be null");
assertEquals(input, result.toString(), "Returned URL should match input");
} catch (MalformedURLException e) {
fail("Exception " + e.getMessage());
}
}


// null input
@Test
public void testNullUrl() {
MalformedURLException exception = assertThrows(MalformedURLException.class, () -> {
URLUtil.create(null);
});
assertTrue(exception.getMessage().contains("null or empty"), "Error message should indicate null or empty input");
}

//empty string input
@Test
public void testEmptyUrl() {
MalformedURLException exception = assertThrows(MalformedURLException.class, () -> {
URLUtil.create(" ");
});
assertTrue(exception.getMessage().contains("null or empty"), "Error message ");
}

// URI without scheme
@Test
public void testUriMissingScheme() {
MalformedURLException exception = assertThrows(MalformedURLException.class, () -> {
URLUtil.create("www.example.com");
});
assertTrue(exception.getMessage().contains("not absolute"), "URI is not absolute");
}

// URI with scheme but missing host
@Test
public void testUriMissingHost() {
MalformedURLException exception = assertThrows(MalformedURLException.class, () -> {
URLUtil.create("mailto:[email protected]");
});
assertTrue(exception.getMessage().contains("must include both scheme and host"), "Error message should mention scheme and host");
}

// malformed syntax
@Test
public void testMalformedSyntax() {
MalformedURLException exception = assertThrows(MalformedURLException.class, () -> {
URLUtil.create("http://[invalid-url]");
});
assertTrue(exception.getMessage().contains("Invalid URI syntax"), " URI syntax error");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments on the Tests can be removed, the name of the method is selfdescribing.
If you think that the name is not sufficient, you can use JUnit annotation @DisplayName to give them better names. But in general it is enough. Here it is enough imho.
The tests look good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I removed it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jenilpatel264 what we meant was error messages on the tests, not code comments

}