Skip to content

Commit

Permalink
fix: use dedicated exception for Location validation (#11367)
Browse files Browse the repository at this point in the history
Fixes #11302
  • Loading branch information
Denis committed Jul 8, 2021
1 parent 7e89dfe commit 6a63c04
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2000-2021 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package com.vaadin.flow.router;

/**
* Thrown to indicate that a {@link Location} instance is invalid.
*
* @author Vaadin Ltd
* @since
*
* @see Location
*/
public class InvalidLocationException extends RuntimeException {

/**
* Creates a new exception with the specified detail message.
*
* @param message
* the detail message. The detail message is saved for later
* retrieval by the {@link #getMessage()} method.
*/
public InvalidLocationException(String message) {
super(message);
}

/**
* Constructs a new runtime exception with the specified detail message and
* cause.
*
* @param message
* the detail message (which is saved for later retrieval by the
* {@link #getMessage()} method).
* @param cause
* the cause (which is saved for later retrieval by the
* {@link #getCause()} method). (A <tt>null</tt> value is
* permitted, and indicates that the cause is nonexistent or
* unknown.)
*/
public InvalidLocationException(String message, Throwable cause) {
super(message, cause);
}
}
19 changes: 12 additions & 7 deletions flow-server/src/main/java/com/vaadin/flow/router/Location.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ public class Location implements Serializable {
*
* @param location
* the relative location or <code>null</code> which is
* interpreted as <code>""</code>
* interpreted as <code>""</code>]
* @throws InvalidLocationException
* If the given string cannot be used for the {@link Location}
*/
public Location(String location) {
public Location(String location) throws InvalidLocationException {
this(parsePath(ensureRelativeNonNull(location), true),
parseParams(ensureRelativeNonNull(location)));
}
Expand Down Expand Up @@ -89,8 +91,11 @@ private static String ensureRelativeNonNull(String location) {
* query parameters information, not {@code null}
* @throws IllegalArgumentException
* if location string contains query parameters inside
* @throws InvalidLocationException
* If the given string cannot be used for the {@link Location}
*/
public Location(String location, QueryParameters queryParameters) {
public Location(String location, QueryParameters queryParameters)
throws InvalidLocationException {
this(parsePath(ensureRelativeNonNull(location), false),
queryParameters);
}
Expand Down Expand Up @@ -298,17 +303,17 @@ private static void verifyRelativePath(String path) {
if (uri.isAbsolute()) {
// "A URI is absolute if, and only if, it has a scheme
// component"
throw new IllegalArgumentException(
throw new InvalidLocationException(
"Relative path cannot contain an URI scheme");
} else if (uri.getPath().startsWith("/")) {
throw new IllegalArgumentException(
throw new InvalidLocationException(
"Relative path cannot start with /");
} else if (hasIncorrectParentSegments(uri.getRawPath())) {
throw new IllegalArgumentException(
throw new InvalidLocationException(
"Relative path cannot contain .. segments");
}
} catch (URISyntaxException | UnsupportedEncodingException e) {
throw new IllegalArgumentException("Cannot parse path: " + path, e);
throw new InvalidLocationException("Cannot parse path: " + path, e);
}

// All is OK if we get here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.Marker;
import org.slf4j.MarkerFactory;

import com.vaadin.flow.router.InvalidLocationException;

/**
* The default implementation of {@link ErrorHandler}.
Expand All @@ -28,10 +32,17 @@
public class DefaultErrorHandler implements ErrorHandler {
@Override
public void error(ErrorEvent event) {
Throwable t = findRelevantThrowable(event.getThrowable());
Throwable throwable = findRelevantThrowable(event.getThrowable());

// print the error on console
getLogger().error("", t);
Marker marker = MarkerFactory.getMarker("INVALID_LOCATION");
if (throwable instanceof InvalidLocationException) {
if (getLogger().isWarnEnabled(marker)) {
getLogger().warn(marker, "", throwable);
}
} else {
// print the error on console
getLogger().error("", throwable);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,23 +261,23 @@ public void locationNameShouldBeAbleToHaveDotDot() {

@Test
public void locationShouldBeRelative() {
expectedEx.expect(IllegalArgumentException.class);
expectedEx.expect(InvalidLocationException.class);
expectedEx.expectMessage("Relative path cannot contain .. segments");

new Location("../element");
}

@Test
public void locationShouldNotEndWithDotDotSegment() {
expectedEx.expect(IllegalArgumentException.class);
expectedEx.expect(InvalidLocationException.class);
expectedEx.expectMessage("Relative path cannot contain .. segments");

new Location("element/..");
}

@Test
public void dotDotLocationShouldNotWork() {
expectedEx.expect(IllegalArgumentException.class);
expectedEx.expect(InvalidLocationException.class);
expectedEx.expectMessage("Relative path cannot contain .. segments");

new Location("..");
Expand Down
22 changes: 12 additions & 10 deletions flow-server/src/test/java/com/vaadin/flow/router/RouterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.vaadin.flow.router;

import javax.servlet.http.HttpServletResponse;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -30,6 +31,15 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import net.jcip.annotations.NotThreadSafe;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Mockito;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.ComponentUtil;
import com.vaadin.flow.component.HasComponents;
Expand All @@ -50,20 +60,12 @@
import com.vaadin.flow.router.internal.HasUrlParameterFormat;
import com.vaadin.flow.router.internal.RouteUtil;
import com.vaadin.flow.server.InvalidRouteConfigurationException;
import com.vaadin.flow.server.InvalidRouteLayoutConfigurationException;
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
import com.vaadin.flow.shared.Registration;

import elemental.json.Json;
import elemental.json.JsonObject;
import net.jcip.annotations.NotThreadSafe;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Mockito;

import static com.vaadin.flow.router.internal.RouteModelTest.parameters;
import static com.vaadin.flow.router.internal.RouteModelTest.varargs;
Expand Down Expand Up @@ -1664,7 +1666,7 @@ public void resolveNavigation_pathContainsDots_dotSegmentIsNotParentReference_no
// doesn't throw
}

@Test(expected = IllegalArgumentException.class)
@Test(expected = InvalidLocationException.class)
public void resolveNavigation_pathContainsDots_pathIsRelative_noException() {
router.resolveNavigationTarget("/../dsfsdfsdf", Collections.emptyMap());
// Location explicitly disallows ".." segments
Expand Down

0 comments on commit 6a63c04

Please sign in to comment.