fix: prevent passing bad character to dev-server #11099

pleku · 2021-05-27T18:02:51Z

The webpack dev-server does not escape " character, as it is not valid
URL. This limitation was not checked when passing request to it via
DevModeHandlerImpl.

flow-server/src/main/java/com/vaadin/flow/server/HandlerHelper.java

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

caalador

just needs running formatter:format

pleku · 2021-05-28T11:52:06Z

Formatter run and hopefully ccdm tests pass this time too

vaadin-bot · 2021-05-28T12:01:48Z

SonarQube analysis reported 7 issues

HandlerHelper.java#L408: Define and throw a dedicated exception instead of using a generic one.
DevModeHandlerImpl.java#L1: Class com.vaadin.base.devserver.DevModeHandlerImpl defines non-transient non-serializable instance field devServerStartFuture
DevModeHandlerImpl.java#L157: Make "devServerStartFuture" transient or serializable.
DevModeHandlerImpl.java#L249: Null passed for non-null parameter of java.util.concurrent.CompletableFuture.getNow(Object) in com.vaadin.base.devserver.DevModeHandlerImpl.handleRequest(VaadinSession, VaadinRequest, VaadinResponse)
DevModeHandlerImpl.java#L581: Remove this unused private "checkPort" method.
DevModeHandlerImpl.java#L718: Either re-interrupt this method or rethrow the "InterruptedException".
HandlerHelper.java#L420: Public static com.vaadin.flow.server.HandlerHelper.getPublicResources() may expose internal representation by returning HandlerHelper.publicResources

vaadin-bot · 2021-05-31T05:55:18Z

Hi @pleku , this commit cannot be picked to 7.0 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick f18ea9a
error: could not apply f18ea9a... fix: prevent passing bad character to dev-server (#11099)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

vaadin-bot · 2021-05-31T05:55:22Z

Hi @pleku , this commit cannot be picked to 6.0 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick f18ea9a
error: could not apply f18ea9a... fix: prevent passing bad character to dev-server (#11099)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

vaadin-bot · 2021-05-31T05:55:26Z

Hi @pleku , this commit cannot be picked to 2.7 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick f18ea9a
error: could not apply f18ea9a... fix: prevent passing bad character to dev-server (#11099)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

vaadin-bot · 2021-05-31T05:55:30Z

Hi @pleku , this commit cannot be picked to 2.6 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick f18ea9a
error: could not apply f18ea9a... fix: prevent passing bad character to dev-server (#11099)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

fluorumlabs

Denylisting is generally a bad approach for dealing with incorrect inputs

fluorumlabs · 2021-05-31T07:55:29Z

vaadin-dev-server/src/main/java/com/vaadin/base/devserver/DevModeHandlerImpl.java

@@ -393,6 +398,7 @@ private boolean checkWebpackConnection() {
    @Override
    public HttpURLConnection prepareConnection(String path, String method)
            throws IOException {
+        // path should have been checked at this point for any outside requests
        URL uri = new URL(WEBPACK_HOST + ":" + getPort() + path);


According to JavaDoc, the recommended way of dealing with encoding is to use new URI(...).toURL():

Note, the java.net.URI class does perform escaping of its
component fields in certain circumstances. The recommended way
to manage the encoding and decoding of URLs is to use java.net.URI,
and to convert between these two classes using toURI() and
toURL().

It would make sense, yes. But the code needs more cleanup to be able to do this properly.
#11117

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

vaadin-bot · 2021-06-07T12:02:51Z

This ticket/PR has been released with platform 21.0.0.alpha3. For prerelease versions, it will be included in its final version.

pleku requested a review from caalador May 27, 2021 18:02

vaadin-bot added the +0.0.1 label May 27, 2021

caalador reviewed May 28, 2021

View reviewed changes

flow-server/src/main/java/com/vaadin/flow/server/HandlerHelper.java Outdated Show resolved Hide resolved

flow-server/src/main/java/com/vaadin/flow/server/HandlerHelper.java Outdated Show resolved Hide resolved

fix: prevent passing bad character to dev-server

2a801c4

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

pleku force-pushed the fix/illegal-char branch from 3952101 to 2a801c4 Compare May 28, 2021 09:52

pleku requested a review from caalador May 28, 2021 09:56

caalador reviewed May 28, 2021

View reviewed changes

chore: run formatter

eb24d8c

pleku requested a review from caalador May 28, 2021 11:51

pleku added target/2.6 labels May 28, 2021

caalador approved these changes May 28, 2021

View reviewed changes

caalador merged commit f18ea9a into master May 31, 2021

caalador deleted the fix/illegal-char branch May 31, 2021 05:50

vaadin-bot added the need to pick manually 7.0 label May 31, 2021

vaadin-bot added the need to pick manually 6.0 label May 31, 2021

vaadin-bot added the need to pick manually 2.7 label May 31, 2021

vaadin-bot added the need to pick manually 2.6 label May 31, 2021

pleku added a commit that referenced this pull request May 31, 2021

fix: prevent passing bad character to wp (#11099)

c6d702a

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

pleku added a commit that referenced this pull request May 31, 2021

fix: prevent passing bad character to wp (#11099)

ecae31c

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

pleku added a commit that referenced this pull request May 31, 2021

fix: prevent passing bad character to wp (#11099)

ee71a2a

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

pleku added a commit that referenced this pull request May 31, 2021

fix: prevent passing bad character to wp (#11099)

d0044ff

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

pleku added cherry-picked-2.6 cherry-picked-2.7 cherry-picked-6.0 cherry-picked-7.0 and removed need to pick manually 2.6 need to pick manually 2.7 need to pick manually 6.0 need to pick manually 7.0 labels May 31, 2021

fluorumlabs reviewed May 31, 2021

View reviewed changes

pleku mentioned this pull request May 31, 2021

refactor DevModeHandler to properly check any url before passing it to webpack #11117

Open

pleku added a commit that referenced this pull request May 31, 2021

fix: prevent passing bad character to wp (#11099)

afb36c5

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

pleku added a commit that referenced this pull request May 31, 2021

fix: prevent passing bad character to wp (#11099)

7ac67cb

The webpack dev-server does not escape " character, as it is not valid URL. This limitation was not checked when passing request to it via DevModeHandlerImpl.

vaadin-bot added prerelease version for platform 21.0.0 Released with Platform 21.0.0.alpha3 labels Jun 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prevent passing bad character to dev-server #11099

fix: prevent passing bad character to dev-server #11099

pleku commented May 27, 2021

caalador left a comment

pleku commented May 28, 2021

vaadin-bot commented May 28, 2021

vaadin-bot commented May 31, 2021

vaadin-bot commented May 31, 2021

vaadin-bot commented May 31, 2021

vaadin-bot commented May 31, 2021

fluorumlabs left a comment

fluorumlabs May 31, 2021

pleku May 31, 2021 •

edited

Loading

vaadin-bot commented Jun 7, 2021

fix: prevent passing bad character to dev-server #11099

fix: prevent passing bad character to dev-server #11099

Conversation

pleku commented May 27, 2021

caalador left a comment

Choose a reason for hiding this comment

pleku commented May 28, 2021

vaadin-bot commented May 28, 2021

vaadin-bot commented May 31, 2021

vaadin-bot commented May 31, 2021

vaadin-bot commented May 31, 2021

vaadin-bot commented May 31, 2021

fluorumlabs left a comment

Choose a reason for hiding this comment

fluorumlabs May 31, 2021

Choose a reason for hiding this comment

pleku May 31, 2021 • edited Loading

Choose a reason for hiding this comment

vaadin-bot commented Jun 7, 2021

pleku May 31, 2021 •

edited

Loading