Skip to content

Update Parser.php - #110

Merged
jmglsn merged 1 commit into
stomp-php:masterfrom
riven8192:patch-1
Sep 5, 2018
Merged

Update Parser.php#110
jmglsn merged 1 commit into
stomp-php:masterfrom
riven8192:patch-1

Conversation

@riven8192

Copy link
Copy Markdown
Contributor

in regex "[....]" denotes a list of chars, hence the pattern "[\r?\n]" is incorrect, as it will split on: '\r', '?' and '\n', while the intention was most likely: split on "\r\n" or "\n", which would have the regex pattern: "(\r?\n)"

in regex "[....]" denotes a list of chars, hence the pattern "[\r?\n]" is incorrect, as it will split on: '\r', '?' and '\n', while the intention was most likely: split on "\r\n" or "\n", which would have the regex pattern: "(\r?\n)"
@riven8192

Copy link
Copy Markdown
Contributor Author

We ran into this issue, because we have header-values with '\r' chars (no '\n' values), which was incorrectly interpreted as a delimiter of the header-value.

@staabm

staabm commented Aug 17, 2018

Copy link
Copy Markdown
Member

A unit test would be great

@riven8192

riven8192 commented Aug 17, 2018

Copy link
Copy Markdown
Contributor Author

I'm sorry, I'm not a PHP developer - I ran into this issue when a Java application interfaced with a PHP application over Stomp. I cannot write a PHP unit-test in a reasonable timeframe, but I wrote a small snippet for you, that showcases the issue (albeit in java -- it should translate trivially to PHP)

`
public static void test() {
String input = "aaa \r bbb ? ccc \r\n ddd";

	String oldOutput = input.replaceAll("[\r?\n]+", "[NEWLINE]");
	String newOutput = input.replaceAll("(\r?\n)+", "[NEWLINE]");

	boolean oldPatternMatches = oldOutput.equals("aaa [NEWLINE] bbb [NEWLINE] ccc [NEWLINE] ddd");
	boolean newPatternMatches = newOutput.equals("aaa \r bbb ? ccc [NEWLINE] ddd");

	System.out.println(oldPatternMatches); // true
	System.out.println(newPatternMatches); // true
}

`

I hope this helps.... somewhat :)

@riven8192

Copy link
Copy Markdown
Contributor Author

As a simple check, please verify that the PHP regex parser indeed splits on the '?' character with the current regex-pattern, as showcased in the java snippet.

@jmglsn

jmglsn commented Sep 5, 2018

Copy link
Copy Markdown
Member

@riven8192 Sorry for the huge delay, I'll check your PR this evening and also try to add a test case.

@jmglsn jmglsn self-assigned this Sep 5, 2018
jmglsn added a commit that referenced this pull request Sep 5, 2018
@jmglsn

jmglsn commented Sep 5, 2018

Copy link
Copy Markdown
Member

Thank you for the PR 👍

@jmglsn
jmglsn merged commit ef0dbac into stomp-php:master Sep 5, 2018
jmglsn added a commit that referenced this pull request Sep 5, 2018
@jmglsn

jmglsn commented Sep 5, 2018

Copy link
Copy Markdown
Member

We released version 4.4.1 containing the fix.

@riven8192
riven8192 deleted the patch-1 branch September 5, 2018 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants