[GitHub] cnsgithub commented on issue #27: MYFACES-4266: Ajax update fails due to invalid characters in response XML (DoS)
cnsgithub commented on issue #27: MYFACES-4266: Ajax update fails due to invalid characters in response XML (DoS) URL: https://github.com/apache/myfaces/pull/27#issuecomment-441461685 > 1. I think we should overwrite all #write methods, to cover 100% all cases?! Probably also writeText? I think the remaining `writeXXX` methods operate on a higher abstraction level and internally call the low level methods like the ones that have already been overridden. The provided unit test is already testing `writeText` to verify this assumption. > 2. all #write methods should call the same super.write - currently the write(String) calls super.write(char[]) to avoid confusion (and maybe but unlikely bugs) I am a bit confused as this statement seems to be in contrast to what you have written in your first review. Afterwards I changed the behavior in https://github.com/apache/myfaces/pull/27/commits/c89e67fd1f5c9456f53f3a3c13b5123da510b632 to fit your requirement. > JFYI: you can also use a for-each loop when looping over arrays, we just avoid it on ArrayLists, to avoid a iterator instance (ArrayLists are used for component lists e.g. and the component tree is traversed very often) Ok, good to know. However, in this special case it's probably necessary to have a counter since we don't want to copy the array, instead we want to modify the contents of the existing array. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (MYFACES-4266) Ajax update fails due to invalid characters in response XML (DoS)
[
https://issues.apache.org/jira/browse/MYFACES-4266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16698244#comment-16698244
]
ASF GitHub Bot commented on MYFACES-4266:
-
cnsgithub commented on issue #27: MYFACES-4266: Ajax update fails due to
invalid characters in response XML (DoS)
URL: https://github.com/apache/myfaces/pull/27#issuecomment-441461685
> 1. I think we should overwrite all #write methods, to cover 100% all
cases?! Probably also writeText?
I think the remaining `writeXXX` methods operate on a higher abstraction
level and internally call the low level methods like the ones that have already
been overridden. The provided unit test is already testing `writeText` to
verify this assumption.
> 2. all #write methods should call the same super.write - currently the
write(String) calls super.write(char[]) to avoid confusion (and maybe but
unlikely bugs)
I am a bit confused as this statement seems to be in contrast to what you
have written in your first review. Afterwards I changed the behavior in
https://github.com/apache/myfaces/pull/27/commits/c89e67fd1f5c9456f53f3a3c13b5123da510b632
to fit your requirement.
> JFYI: you can also use a for-each loop when looping over arrays, we just
avoid it on ArrayLists, to avoid a iterator instance (ArrayLists are used for
component lists e.g. and the component tree is traversed very often)
Ok, good to know. However, in this special case it's probably necessary to
have a counter since we don't want to copy the array, instead we want to modify
the contents of the existing array.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Ajax update fails due to invalid characters in response XML (DoS)
> -
>
> Key: MYFACES-4266
> URL: https://issues.apache.org/jira/browse/MYFACES-4266
> Project: MyFaces Core
> Issue Type: Bug
>Affects Versions: 2.3.2
> Environment: jetty 9.4.14.v20181114
> JDK 10
>Reporter: cnsgithub
>Priority: Major
>
> I noticed that the {{}} update fails when the updated form contains
> unicode characters, which are not allowed in the [XML 1.0
> spec|https://www.w3.org/TR/REC-xml/#charsets].
> h2. Expected Behaviour
> If the update response contains characters that are not allowed in XML, they
> should be filtered by MyFaces before writing the response.
> h2. Actual Behaviour
> Some illegal XML characters are not filtered and therefore the browser fails
> to parse the response.
> h2. Steps to reproduce
> I created a small github project to reproduce this behaviour:
> [https://github.com/cnsgithub/mojarra-ajax/tree/myfaces] (branch myfaces)
> To reproduce:
> - {{git clone [https://github.com/cnsgithub/mojarra-ajax]}}
> - {{git checkout myfaces}}
> - run {{mvn clean package jetty:run}}
> - after the server has started, open [http://localhost:8080/index.xhtml]
> - Click the button, the error should occur
> The issue also occurs with user supplied inputs:
> - open [http://localhost:8080/input.xhtml]
> - Paste the characters from the {{illegal-xml-chars.txt}} file into the
> input field
> - Click the button
> This issue should be addressed with high priority since it is security
> related (might be exploited for Denial of Service).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] tandraschko commented on issue #27: MYFACES-4266: Ajax update fails due to invalid characters in response XML (DoS)
tandraschko commented on issue #27: MYFACES-4266: Ajax update fails due to
invalid characters in response XML (DoS)
URL: https://github.com/apache/myfaces/pull/27#issuecomment-441476483
1) all right, it would be just cool if we would have some more tests which
also covers other #write methods or even writeAttribute - but not required
2) Oh, really sorry - thats my fault because of a too fast review. I
thought that you would wrap a simple char with a string now.
I would just do:
`@Override
public void write(String str, int off, int len) throws IOException
{
super.write(new String(xmlEncode(str.toCharArray())), off, len);
}`
I just wonder if we should replace the invalid char by a blank instead of
empty? Not sure...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Commented] (MYFACES-4266) Ajax update fails due to invalid characters in response XML (DoS)
[
https://issues.apache.org/jira/browse/MYFACES-4266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16698329#comment-16698329
]
ASF GitHub Bot commented on MYFACES-4266:
-
tandraschko commented on issue #27: MYFACES-4266: Ajax update fails due to
invalid characters in response XML (DoS)
URL: https://github.com/apache/myfaces/pull/27#issuecomment-441476483
1) all right, it would be just cool if we would have some more tests which
also covers other #write methods or even writeAttribute - but not required
2) Oh, really sorry - thats my fault because of a too fast review. I
thought that you would wrap a simple char with a string now.
I would just do:
`@Override
public void write(String str, int off, int len) throws IOException
{
super.write(new String(xmlEncode(str.toCharArray())), off, len);
}`
I just wonder if we should replace the invalid char by a blank instead of
empty? Not sure...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Ajax update fails due to invalid characters in response XML (DoS)
> -
>
> Key: MYFACES-4266
> URL: https://issues.apache.org/jira/browse/MYFACES-4266
> Project: MyFaces Core
> Issue Type: Bug
>Affects Versions: 2.3.2
> Environment: jetty 9.4.14.v20181114
> JDK 10
>Reporter: cnsgithub
>Priority: Major
>
> I noticed that the {{}} update fails when the updated form contains
> unicode characters, which are not allowed in the [XML 1.0
> spec|https://www.w3.org/TR/REC-xml/#charsets].
> h2. Expected Behaviour
> If the update response contains characters that are not allowed in XML, they
> should be filtered by MyFaces before writing the response.
> h2. Actual Behaviour
> Some illegal XML characters are not filtered and therefore the browser fails
> to parse the response.
> h2. Steps to reproduce
> I created a small github project to reproduce this behaviour:
> [https://github.com/cnsgithub/mojarra-ajax/tree/myfaces] (branch myfaces)
> To reproduce:
> - {{git clone [https://github.com/cnsgithub/mojarra-ajax]}}
> - {{git checkout myfaces}}
> - run {{mvn clean package jetty:run}}
> - after the server has started, open [http://localhost:8080/index.xhtml]
> - Click the button, the error should occur
> The issue also occurs with user supplied inputs:
> - open [http://localhost:8080/input.xhtml]
> - Paste the characters from the {{illegal-xml-chars.txt}} file into the
> input field
> - Click the button
> This issue should be addressed with high priority since it is security
> related (might be exploited for Denial of Service).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] cnsgithub commented on issue #27: MYFACES-4266: Ajax update fails due to invalid characters in response XML (DoS)
cnsgithub commented on issue #27: MYFACES-4266: Ajax update fails due to
invalid characters in response XML (DoS)
URL: https://github.com/apache/myfaces/pull/27#issuecomment-441527197
> 1. all right, it would be just cool if we would have some more tests which
also covers other #write methods or even writeAttribute - but not required
Provided another test for `writeAttribute`.
> 2. Oh, really sorry - thats my fault because of a too fast review. I
thought that you would wrap a simple char with a string now.
>I would just do:
>
> ` @Override public void write(String str, int off, int len) throws
IOException { super.write(new String(xmlEncode(str.toCharArray())), off, len);
}`
Done.
> I just wonder if we should replace the invalid char by a blank instead of
empty? Not sure...
Also considered this. However, it would complicate things since array
lengths might change then. When looking at OWASP's encoder you'll find they're
also replacing illegal characters by spaces.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Commented] (MYFACES-4266) Ajax update fails due to invalid characters in response XML (DoS)
[
https://issues.apache.org/jira/browse/MYFACES-4266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16698499#comment-16698499
]
ASF GitHub Bot commented on MYFACES-4266:
-
cnsgithub commented on issue #27: MYFACES-4266: Ajax update fails due to
invalid characters in response XML (DoS)
URL: https://github.com/apache/myfaces/pull/27#issuecomment-441527197
> 1. all right, it would be just cool if we would have some more tests which
also covers other #write methods or even writeAttribute - but not required
Provided another test for `writeAttribute`.
> 2. Oh, really sorry - thats my fault because of a too fast review. I
thought that you would wrap a simple char with a string now.
>I would just do:
>
> ` @Override public void write(String str, int off, int len) throws
IOException { super.write(new String(xmlEncode(str.toCharArray())), off, len);
}`
Done.
> I just wonder if we should replace the invalid char by a blank instead of
empty? Not sure...
Also considered this. However, it would complicate things since array
lengths might change then. When looking at OWASP's encoder you'll find they're
also replacing illegal characters by spaces.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Ajax update fails due to invalid characters in response XML (DoS)
> -
>
> Key: MYFACES-4266
> URL: https://issues.apache.org/jira/browse/MYFACES-4266
> Project: MyFaces Core
> Issue Type: Bug
>Affects Versions: 2.3.2
> Environment: jetty 9.4.14.v20181114
> JDK 10
>Reporter: cnsgithub
>Priority: Major
>
> I noticed that the {{}} update fails when the updated form contains
> unicode characters, which are not allowed in the [XML 1.0
> spec|https://www.w3.org/TR/REC-xml/#charsets].
> h2. Expected Behaviour
> If the update response contains characters that are not allowed in XML, they
> should be filtered by MyFaces before writing the response.
> h2. Actual Behaviour
> Some illegal XML characters are not filtered and therefore the browser fails
> to parse the response.
> h2. Steps to reproduce
> I created a small github project to reproduce this behaviour:
> [https://github.com/cnsgithub/mojarra-ajax/tree/myfaces] (branch myfaces)
> To reproduce:
> - {{git clone [https://github.com/cnsgithub/mojarra-ajax]}}
> - {{git checkout myfaces}}
> - run {{mvn clean package jetty:run}}
> - after the server has started, open [http://localhost:8080/index.xhtml]
> - Click the button, the error should occur
> The issue also occurs with user supplied inputs:
> - open [http://localhost:8080/input.xhtml]
> - Paste the characters from the {{illegal-xml-chars.txt}} file into the
> input field
> - Click the button
> This issue should be addressed with high priority since it is security
> related (might be exploited for Denial of Service).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] cnsgithub edited a comment on issue #27: MYFACES-4266: Ajax update fails due to invalid characters in response XML (DoS)
cnsgithub edited a comment on issue #27: MYFACES-4266: Ajax update fails due to
invalid characters in response XML (DoS)
URL: https://github.com/apache/myfaces/pull/27#issuecomment-441527197
> 1. all right, it would be just cool if we would have some more tests which
also covers other #write methods or even writeAttribute - but not required
Provided another test for `writeAttribute`.
> 2. Oh, really sorry - thats my fault because of a too fast review. I
thought that you would wrap a simple char with a string now.
>I would just do:
>
> ` @Override public void write(String str, int off, int len) throws
IOException { super.write(new String(xmlEncode(str.toCharArray())), off, len);
}`
Done.
> I just wonder if we should replace the invalid char by a blank instead of
empty? Not sure...
Also considered this. However, it would complicate things since array
lengths might change then. When looking at OWASP's encoder you'll find they
also replace illegal characters by spaces.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Commented] (MYFACES-4266) Ajax update fails due to invalid characters in response XML (DoS)
[
https://issues.apache.org/jira/browse/MYFACES-4266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16698501#comment-16698501
]
ASF GitHub Bot commented on MYFACES-4266:
-
cnsgithub edited a comment on issue #27: MYFACES-4266: Ajax update fails due to
invalid characters in response XML (DoS)
URL: https://github.com/apache/myfaces/pull/27#issuecomment-441527197
> 1. all right, it would be just cool if we would have some more tests which
also covers other #write methods or even writeAttribute - but not required
Provided another test for `writeAttribute`.
> 2. Oh, really sorry - thats my fault because of a too fast review. I
thought that you would wrap a simple char with a string now.
>I would just do:
>
> ` @Override public void write(String str, int off, int len) throws
IOException { super.write(new String(xmlEncode(str.toCharArray())), off, len);
}`
Done.
> I just wonder if we should replace the invalid char by a blank instead of
empty? Not sure...
Also considered this. However, it would complicate things since array
lengths might change then. When looking at OWASP's encoder you'll find they
also replace illegal characters by spaces.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Ajax update fails due to invalid characters in response XML (DoS)
> -
>
> Key: MYFACES-4266
> URL: https://issues.apache.org/jira/browse/MYFACES-4266
> Project: MyFaces Core
> Issue Type: Bug
>Affects Versions: 2.3.2
> Environment: jetty 9.4.14.v20181114
> JDK 10
>Reporter: cnsgithub
>Priority: Major
>
> I noticed that the {{}} update fails when the updated form contains
> unicode characters, which are not allowed in the [XML 1.0
> spec|https://www.w3.org/TR/REC-xml/#charsets].
> h2. Expected Behaviour
> If the update response contains characters that are not allowed in XML, they
> should be filtered by MyFaces before writing the response.
> h2. Actual Behaviour
> Some illegal XML characters are not filtered and therefore the browser fails
> to parse the response.
> h2. Steps to reproduce
> I created a small github project to reproduce this behaviour:
> [https://github.com/cnsgithub/mojarra-ajax/tree/myfaces] (branch myfaces)
> To reproduce:
> - {{git clone [https://github.com/cnsgithub/mojarra-ajax]}}
> - {{git checkout myfaces}}
> - run {{mvn clean package jetty:run}}
> - after the server has started, open [http://localhost:8080/index.xhtml]
> - Click the button, the error should occur
> The issue also occurs with user supplied inputs:
> - open [http://localhost:8080/input.xhtml]
> - Paste the characters from the {{illegal-xml-chars.txt}} file into the
> input field
> - Click the button
> This issue should be addressed with high priority since it is security
> related (might be exploited for Denial of Service).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
