You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2021/05/11 08:22:24 UTC
[GitHub] [cxf] eberntsen opened a new pull request #795: [CXF-8535] Query missing from signature request-target
eberntsen opened a new pull request #795:
URL: https://github.com/apache/cxf/pull/795
Added query component to request-target in interceptor/filter.
Added tests that use the interceptor/filter for the spec tests. The existing tests only covered the inner objects (MessageSigner/MessageVerifier) and thus missed this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] eberntsen commented on pull request #795: [CXF-8535] Query missing from signature request-target
Posted by GitBox <gi...@apache.org>.
eberntsen commented on pull request #795:
URL: https://github.com/apache/cxf/pull/795#issuecomment-849457831
Fixed checkstyle issues now.
The previous unit tests didn't fail previously because they didn't target any classes that had any issues. They don't fail now either because I haven't touched the existing methods, only added methods that tested other classes. The problem here was that the tests only manually tested the inner Signer/Verifier classes, but the issue was with the Interceptor/Filter layer. The Interceptor/Filter didn't give the inner classes the same input as the tests did.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea commented on pull request #795: [CXF-8535] Query missing from signature request-target
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #795:
URL: https://github.com/apache/cxf/pull/795#issuecomment-848928940
Please correct the checkstyle failures:
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/utils/SignatureHeaderUtils.java:109:9: 'if' is not followed by whitespace. [WhitespaceAround]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/CreateSignatureInterceptor.java:32:1: Wrong order for 'javax.annotation.Priority' import. [ImportOrder]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/CreateSignatureInterceptor.java:45:1: Wrong order for 'java.io.IOException' import. [ImportOrder]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/CreateSignatureInterceptor.java:87: Line is longer than 120 characters (found 133). [LineLength]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/main/java/org/apache/cxf/rs/security/httpsignature/filters/VerifySignatureFilter.java:23:1: Wrong order for 'java.io.ByteArrayInputStream' import. [ImportOrder]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:33:1: Wrong order for 'org.powermock.core.classloader.annotations.PrepareForTest' import. [ImportOrder]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:36:1: Wrong order for 'javax.ws.rs.client.ClientRequestContext' import. [ImportOrder]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:41:1: Wrong order for 'java.io.ByteArrayInputStream' import. [ImportOrder]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:58:8: Unused import - java.util.stream.Collectors. [UnusedImports]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:61:53: Using the '.*' form of import should be avoided - org.powermock.api.mockito.PowerMockito.*. [AvoidStarImport]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:169: Line is longer than 120 characters (found 124). [LineLength]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:244: Line is longer than 120 characters (found 124). [LineLength]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:321: Line is longer than 120 characters (found 124). [LineLength]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:328: Line is longer than 120 characters (found 133). [LineLength]
[ERROR] /home/coheig/src/apache/cxf/rt/rs/security/http-signature/src/test/java/org/apache/cxf/rs/security/httpsignature/SpecExamplesTest.java:337: Line is longer than 120 characters (found 145). [LineLength]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] eberntsen edited a comment on pull request #795: [CXF-8535] Query missing from signature request-target
Posted by GitBox <gi...@apache.org>.
eberntsen edited a comment on pull request #795:
URL: https://github.com/apache/cxf/pull/795#issuecomment-849457831
Fixed checkstyle issues now.
The previous unit tests didn't fail previously because they didn't target any classes that had any issues. They don't fail now either because I haven't touched the existing methods, only added methods that tested other classes. The problem here was that the tests only manually tested the inner Signer/Verifier classes with test data that matches the spec, but was different from what they would receive from the Interceptor/Filter. The issue was with the Interceptor/Filter layer. The Interceptor/Filter didn't give the inner classes the same input as the tests did.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea commented on pull request #795: [CXF-8535] Query missing from signature request-target
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #795:
URL: https://github.com/apache/cxf/pull/795#issuecomment-848930820
Overall LGTM, but I don't understand why the previous unit tests didn't fail, given that they were including a query parameter and we were testing against a Signature expected header, once the patch changed it to also sign the query shouldn't the signature header have been different?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] eberntsen commented on pull request #795: [CXF-8535] Query missing from signature request-target
Posted by GitBox <gi...@apache.org>.
eberntsen commented on pull request #795:
URL: https://github.com/apache/cxf/pull/795#issuecomment-844985877
@coheigea can you please have a look at this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea merged pull request #795: [CXF-8535] Query missing from signature request-target
Posted by GitBox <gi...@apache.org>.
coheigea merged pull request #795:
URL: https://github.com/apache/cxf/pull/795
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org