3
\$\begingroup\$

My last project shared here was apparently too complex so I figured I'd share something really simple

Here's a UriValidator. Its purpose is to validate URI paths and schemes. Admittedly, one would expect it to validate entire URIs too, but I didn't need it, and SchemeAndPathValidator was too clumsy a name

package com.example.dynamicgateway.util;

import java.util.regex.Pattern;

/**
 * Utility class for validating URI schemes and paths. Though overall it respects the
 * RFC 3986 specification, this class takes the liberty of diverging from it in the following
 * ways:
 * <p>
 * 1. A scheme's definition is extended to include the characters separating it from the authority,
 * specifically a colon and two forward slashes.
 * For example, in {@code http://example.com} the scheme would be {@code http://}, not {@code http}.
 * The latter would <em>not</em> be considered a valid scheme by this class
 * <p>
 * 2. Trailing slashes are <em>not</em> allowed for paths. A special case for this rule is that
 * {@code /} is <i>not</i> considered a valid path
 *
 * @see <a href="https://datatracker.ietf.org/doc/html/rfc3986">RFC 3986</a>
 */
public class UriValidator {
    /**
     * Checks the validity of a URI path throwing an exception if the check fails
     *
     * @param path URI path for validation
     * @throws IllegalArgumentException if the passed path is invalid
     */
    public static void requireValidPath(String path) {
        boolean matchesPathPattern = isValidPath(path);
        if (!matchesPathPattern) {
            throw new IllegalArgumentException("Invalid path: " + path);
        }
    }

    /**
     * Checks if a URI path is valid
     *
     * @param path URI path
     * @return {@code true} if the path is valid, {@code false} otherwise
     */
    public static boolean isValidPath(String path) {
        return Pattern.matches("/[^?#]+[^/?#]|", path);
    }

    /**
     * Checks the validity of a URI scheme throwing an exception if the check fails
     *
     * @param scheme URI scheme for validation
     * @throws IllegalArgumentException if the passed scheme is invalid
     */
    public static void requireValidScheme(String scheme) {
        boolean matchesSchemePattern = isValidScheme(scheme);
        if (!matchesSchemePattern) {
            throw new IllegalArgumentException("Invalid scheme: " + scheme);
        }
    }

    /**
     * Checks if a URI scheme is valid
     *
     * @param scheme URI scheme
     * @return {@code true} if the scheme is valid, {@code false} otherwise
     */
    public static boolean isValidScheme(String scheme) {
        return Pattern.matches("[A-Za-z][A-Za-z+.-]+://", scheme);
    }
}
package com.example.dynamicgateway.util;

import lombok.Getter;
import lombok.experimental.Accessors;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.List;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

@Accessors(fluent = true)
class UriValidatorTest {
    @Getter
    private static final List<String> validPaths = List.of(
            "/path", "/path/goes-on", "/p@a$t^h", ""
    );
    @Getter
    private static final List<String> invalidPaths = List.of(
            "path", "path/", "/path/", "/path?", "/"
    );
    @Getter
    private static final List<String> validSchemes = List.of(
            "http://", "lalala://", "Ht+-t.p://"
    );
    @Getter
    private static final List<String> invalidSchemes = List.of(
            "1http://", "https:/", "https//", "https;//"
    );

    @ParameterizedTest
    @MethodSource("validPaths")
    void testIsValidPath_withValidPath(String validPath) {
        assertThat(UriValidator.isValidPath(validPath)).isTrue();
    }

    @ParameterizedTest
    @MethodSource("invalidPaths")
    void testIsValidPath_withInvalidPath(String invalidPath) {
        assertThat(UriValidator.isValidPath(invalidPath)).isFalse();
    }

    @ParameterizedTest
    @MethodSource("validPaths")
    void requireValidPath_withValidPath(String validPath) {
        assertThatCode(() -> UriValidator.requireValidPath(validPath)).doesNotThrowAnyException();
    }

    @ParameterizedTest
    @MethodSource("invalidPaths")
    void requireValidPath_withInvalidPath(String invalidPath) {
        assertThatThrownBy(() -> UriValidator.requireValidPath(invalidPath)).isInstanceOf(RuntimeException.class);
    }

    @ParameterizedTest
    @MethodSource("validSchemes")
    void isValidScheme_withValidScheme(String validScheme) {
        assertThat(UriValidator.isValidScheme(validScheme)).isTrue();
    }

    @ParameterizedTest
    @MethodSource("invalidSchemes")
    void isValidScheme_withInvalidScheme(String invalidScheme) {
        assertThat(UriValidator.isValidScheme(invalidScheme)).isFalse();
    }

    @ParameterizedTest
    @MethodSource("validSchemes")
    void requireValidScheme_withValidScheme(String validScheme) {
        assertThatCode(() -> UriValidator.requireValidScheme(validScheme)).doesNotThrowAnyException();
    }

    @ParameterizedTest
    @MethodSource("invalidSchemes")
    void requireValidScheme_withInvalidScheme(String invalidScheme) {
        assertThatThrownBy(() -> UriValidator.requireValidScheme(invalidScheme)).isInstanceOf(RuntimeException.class);
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Missing from the tests: we're not testing schemes such as mailto and tel which don't normally contain /. \$\endgroup\$ Feb 6 at 10:55
  • \$\begingroup\$ I think it is important for you to describe how this differs from java.net.URI. \$\endgroup\$
    – Reinderien
    Feb 6 at 12:46
  • \$\begingroup\$ @Reinderien does it have the methods UriValidator has? I believe it does not \$\endgroup\$
    – Sergey
    Feb 6 at 13:01

1 Answer 1

3
\$\begingroup\$

This class definition starts off well by describing which RFC is being adhered to and what it is trying to do. It is not entirely clear why the slash separator characters would be included as part of the URI-scheme name though. That's more part of a full URI checker.

An extreme nitpick: IETF actually uses the RFC editor sites rather than the data tracker sites as official reference. So, eh, that URI is not valid; ask me how I know ;)


requireValidPath : why not just validatePath? To me require doesn't offer an additional hint that exceptions will be thrown; for that I'd have to look into the doc anyway.


Not sure why the boolean matchesPathPattern is required or why it is called that. In this code it is unspecified how isValidPath does its job; it may well use parsing instead of pattern matching or a library provided function.


isValidPath seems to allow way too much for valid paths, only excluding specific characters and that's it.


In the documentation of the param it sometimes says "for validation" and sometimes it doesn't. As it is clear from context that piece of text may be left out.


In the tests there are checks for RuntimeException while the documentation and code explicitly reads IllegalArgumentException.

Notes

It's a bit weird that full URL's cannot be checked, or why existing code isn't used for this. I'm assuming that it may be enhanced later.

What may confuse users is the depth of testing: these tests only check for validity of the identifiers; they do not check if the scheme or path make sense contextually. I haven't included this in the review as no context is given, which would probably give enough of a hint. Still I would like the word "syntax" in the class description.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you! 1. require*() is a standard idiom for throwing validating methods used in e.g. Objects (at least, I believe so). 2. I figured asserting on the exact exception class would make the tests fragile. I may change the exception type in the future and want to avoid the bother of editing the tests too \$\endgroup\$
    – Sergey
    Feb 7 at 3:14
  • \$\begingroup\$ As for 1. sure, if it is this way on the platform; it is unknown to me though and I cannot find anything using Google (using Java since 1.0)... As for 2. I would at least document that in the test cases. Note that this may be a breaking change for forward compatibility: code that relies on catching an IllegalArgumentException would be bypassed if another RuntimeException is thrown! \$\endgroup\$ Feb 7 at 4:41
  • \$\begingroup\$ Seems restricted mostly to Objects; fair enough I suppose. But I do see no mention of it as best practice anywhere else. \$\endgroup\$ Feb 7 at 4:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.