-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Force Binary Canonicalization #110
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #110 +/- ##
==========================================
+ Coverage 97.46% 97.48% +0.02%
==========================================
Files 12 13 +1
Lines 592 597 +5
==========================================
+ Hits 577 582 +5
Misses 15 15 ☔ View full report in Codecov by Sentry. |
@@ -565,6 +627,20 @@ def compareFiles(filename1, filename2): | |||
lineA == lineB for lineA, lineB in zip(a.readlines(), b.readlines()) | |||
) | |||
|
|||
@staticmethod | |||
def mock_message_content(message): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as2message.content uses mime_to_bytes which will replace with . Different outputs will be like this:
Without Send Message Mock:
b'--===============1167153831119882001==\r\nContent-Type: application/edi-consent\r\nContent-Transfer-Encoding: 7bit\r\nContent-Disposition: attachment; filename="testmessage.edi"\r\n\r\nUNB+UNOA:2+<Sender GLN>:14+<Receiver GLN>:14+140407:0910+5++++1+EANCOM\'\r\nUNH+1+ORDERS:D:96A:UN:EAN008\'\r\nBGM+220+1AA1TEST+9\'\r\nDTM+137:20140407:102\'\r\nDTM+63:20140421:102\'\r\nDTM+64:20140414:102\'\r\nRFF+ADE:1234\'\r\nRFF+PD:1704\'\r\nNAD+BY+5450534000024::9\'\r\nNAD+SU+<Supplier GLN>::9\'\r\nNAD+DP+5450534000109::9+++++++GB\'\r\nNAD+IV+5450534000055::9++AMAZON EU SARL:5 RUE PLAETIS LUXEMBOURG+CO PO BOX 4558+SLOUGH++SL1 0TX+GB\'\r\nRFF+VA:GB727255821\'\r\nCUX+2:EUR:9\'\r\nLIN+1++9783898307529:EN\'\r\nQTY+21:5\'\r\nPRI+AAA:27.5\'\r\nLIN+2++390787706322:UP\'\r\nQTY+21:1\'\r\nPRI+AAA:10.87\'\r\nLIN+3\'\r\nPIA+5+3899408268X-39:SA\'\r\nQTY+21:3\'\r\nPRI+AAA:3.85\'\r\nUNS+S\'\r\nCNT+2:3\'\r\nUNT+26+1\'\r\nUNZ+1+5\'\r\n\r\n--===============1167153831119882001==\r\nContent-Type: application/pkcs7-signature; name="smime.p7s"; smime-type="signed-data"\r\nContent-Disposition: attachment; filename="smime.p7s"\r\nContent-Transfer-Encoding: base64\r\n\r\nMIIE3wYJKoZIhvcNAQcCoIIE0DCCBMwCAQExCzAJBgUrDgMCGgUAMAsGCSqGSIb3DQEHAaCCAqgw\r\nggKkMIIBjAIJAKLCSQTWCttYMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNVBAMMCWFzMmNsaWVudDAe\r\nFw0xODA0MTgwMjQ0MDVaFw0yODA0MTUwMjQ0MDVaMBQxEjAQBgNVBAMMCWFzMmNsaWVudDCCASIw\r\nDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMO0X0wbu159xNFzyEiBT2wmjpPq8icijMqqpk8p\r\nwcENV5WqFbE23MKre/YLAvt1YttFjUVBmAOx5Tq0uPf/efXTlMaCjFKxJMWoaa4VUxXUd4Trioi6\r\ncAFFVVwRJXRnpQ2RUuZ2HHEX7EbB7TFQ/YzPuncmztvypm+duubh9+BBbjAlmLDg76w68l9cMV/E\r\nHS0E05t5Erg6U2jq7xdIgqV3k0ca7W12El6RY4NNH0DdGYD2lw3Y8b27AbsWCrtZORCq8Juv11Qz\r\nHpa8wtkJdot8kmu7JSAX4+arb2XVqfwejgLxJ0HFTL+3T3c7vbSCfrU2ezV81HDNVHSTyHonilkC\r\nAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAlgCZ9H8PxfqG2V4iGYjvP6K2IUW1TNtuuuZYmzM9cNDd\r\nHbfj56k+ARAY4IxeH1KIVQ0f9Vfxwf3q66ZC+UuMe8jrhGNxYaJgTOlzjt7FFOniDyb59qXoSOIo\r\n+KKRTxi/YT8z9z+yHpNqMbnc2/n6E7WwilnBtgZckneOhugzQsBPpnmg0I7gBBPLMnp7zbMni7Yc\r\nD4ZLR4Y4b076vcbSr588a/VudJ3sY133/1X3bqKgkh+4tE88o1YlA1csbOw1ombt05V/qJB2IQy6\r\nFDIjDcgoiRQr/phgDiSuJHEgifkBuFRZl9wZoXEcUrrJiX1Hl0CbSqN0J0FSr58obpSUYjGCAf8w\r\nggH7AgEBMCEwFDESMBAGA1UEAwwJYXMyY2xpZW50AgkAosJJBNYK21gwCQYFKw4DAhoFAKCBtDAY\r\nBgkqhkiG9w0BCQMxCwYJKoZIhvcNAQcBMBwGCSqGSIb3DQEJBTEPFw0yNDAzMjcwODA4MTVaMCMG\r\nCSqGSIb3DQEJBDEWBBTjJrOy5sHEjlsJ17uCi75WEuGWwzBVBgkqhkiG9w0BCQ8xSDBGMAsGCWCG\r\nSAFlAwQBKjALBglghkgBZQMEAQIwCgYIKoZIhvcNAwcwDgYIKoZIhvcNAwICAgCAMA4GCCqGSIb3\r\nDQMEAgIAgDANBgkqhkiG9w0BAQEFAASCAQC3iCVHW+9SdpQdjF29mMe42gPUELo+3UvGHWKlZaeY\r\nUbAlKGjl7OE/ht+bulrmwpzdCvVjoegdFn9LRn2GfgALN6O5D2ktaPX6dli2nZ+F76r/sDJ4JQGM\r\nSoxEz03AbvbSqS25B2AKEYQC/8sOfdlaYP+U2CsRuxJWdQYipxjovUDPADNKjGjZg+jEl8RWkq+Y\r\nRMGQaQL8iCnjtOmH6OWQD6m9B8B48PiBCdi04UwlWJS7s+VAnLETq3NTrk1tav8DmilTcXygFXaM\r\nVYabrGYugb620iuhapusOFvRxV8QpLss5AUvpQO+fnFr1ZKlAVOeNFiCi+cPrDr+ZrXQdhmE\r\n\r\n--===============1167153831119882001==--\r\n'
With sending Message Mock:
b'--===============1167153831119882001==\nContent-Type: application/edi-consent\nContent-Transfer-Encoding: 7bit\nContent-Disposition: attachment; filename="testmessage.edi"\n\nUNB+UNOA:2+<Sender GLN>:14+<Receiver GLN>:14+140407:0910+5++++1+EANCOM\'\nUNH+1+ORDERS:D:96A:UN:EAN008\'\nBGM+220+1AA1TEST+9\'\nDTM+137:20140407:102\'\nDTM+63:20140421:102\'\nDTM+64:20140414:102\'\nRFF+ADE:1234\'\nRFF+PD:1704\'\nNAD+BY+5450534000024::9\'\nNAD+SU+<Supplier GLN>::9\'\nNAD+DP+5450534000109::9+++++++GB\'\nNAD+IV+5450534000055::9++AMAZON EU SARL:5 RUE PLAETIS LUXEMBOURG+CO PO BOX 4558+SLOUGH++SL1 0TX+GB\'\nRFF+VA:GB727255821\'\nCUX+2:EUR:9\'\nLIN+1++9783898307529:EN\'\nQTY+21:5\'\nPRI+AAA:27.5\'\nLIN+2++390787706322:UP\'\nQTY+21:1\'\nPRI+AAA:10.87\'\nLIN+3\'\nPIA+5+3899408268X-39:SA\'\nQTY+21:3\'\nPRI+AAA:3.85\'\nUNS+S\'\nCNT+2:3\'\nUNT+26+1\'\nUNZ+1+5\'\n\n--===============1167153831119882001==\nContent-Type: application/pkcs7-signature; name="smime.p7s"; smime-type="signed-data"\nContent-Disposition: attachment; filename="smime.p7s"\nContent-Transfer-Encoding: base64\n\nMIIE3wYJKoZIhvcNAQcCoIIE0DCCBMwCAQExCzAJBgUrDgMCGgUAMAsGCSqGSIb3DQEHAaCCAqgw\nggKkMIIBjAIJAKLCSQTWCttYMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNVBAMMCWFzMmNsaWVudDAe\nFw0xODA0MTgwMjQ0MDVaFw0yODA0MTUwMjQ0MDVaMBQxEjAQBgNVBAMMCWFzMmNsaWVudDCCASIw\nDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMO0X0wbu159xNFzyEiBT2wmjpPq8icijMqqpk8p\nwcENV5WqFbE23MKre/YLAvt1YttFjUVBmAOx5Tq0uPf/efXTlMaCjFKxJMWoaa4VUxXUd4Trioi6\ncAFFVVwRJXRnpQ2RUuZ2HHEX7EbB7TFQ/YzPuncmztvypm+duubh9+BBbjAlmLDg76w68l9cMV/E\nHS0E05t5Erg6U2jq7xdIgqV3k0ca7W12El6RY4NNH0DdGYD2lw3Y8b27AbsWCrtZORCq8Juv11Qz\nHpa8wtkJdot8kmu7JSAX4+arb2XVqfwejgLxJ0HFTL+3T3c7vbSCfrU2ezV81HDNVHSTyHonilkC\nAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAlgCZ9H8PxfqG2V4iGYjvP6K2IUW1TNtuuuZYmzM9cNDd\nHbfj56k+ARAY4IxeH1KIVQ0f9Vfxwf3q66ZC+UuMe8jrhGNxYaJgTOlzjt7FFOniDyb59qXoSOIo\n+KKRTxi/YT8z9z+yHpNqMbnc2/n6E7WwilnBtgZckneOhugzQsBPpnmg0I7gBBPLMnp7zbMni7Yc\nD4ZLR4Y4b076vcbSr588a/VudJ3sY133/1X3bqKgkh+4tE88o1YlA1csbOw1ombt05V/qJB2IQy6\nFDIjDcgoiRQr/phgDiSuJHEgifkBuFRZl9wZoXEcUrrJiX1Hl0CbSqN0J0FSr58obpSUYjGCAf8w\nggH7AgEBMCEwFDESMBAGA1UEAwwJYXMyY2xpZW50AgkAosJJBNYK21gwCQYFKw4DAhoFAKCBtDAY\nBgkqhkiG9w0BCQMxCwYJKoZIhvcNAQcBMBwGCSqGSIb3DQEJBTEPFw0yNDAzMjcwODA4MTVaMCMG\nCSqGSIb3DQEJBDEWBBTjJrOy5sHEjlsJ17uCi75WEuGWwzBVBgkqhkiG9w0BCQ8xSDBGMAsGCWCG\nSAFlAwQBKjALBglghkgBZQMEAQIwCgYIKoZIhvcNAwcwDgYIKoZIhvcNAwICAgCAMA4GCCqGSIb3\nDQMEAgIAgDANBgkqhkiG9w0BAQEFAASCAQC3iCVHW+9SdpQdjF29mMe42gPUELo+3UvGHWKlZaeY\nUbAlKGjl7OE/ht+bulrmwpzdCvVjoegdFn9LRn2GfgALN6O5D2ktaPX6dli2nZ+F76r/sDJ4JQGM\nSoxEz03AbvbSqS25B2AKEYQC/8sOfdlaYP+U2CsRuxJWdQYipxjovUDPADNKjGjZg+jEl8RWkq+Y\nRMGQaQL8iCnjtOmH6OWQD6m9B8B48PiBCdi04UwlWJS7s+VAnLETq3NTrk1tav8DmilTcXygFXaM\nVYabrGYugb620iuhapusOFvRxV8QpLss5AUvpQO+fnFr1ZKlAVOeNFiCi+cPrDr+ZrXQdhmE\n\n--===============1167153831119882001==--\n'
return boundary + boundary.join(temp) | ||
|
||
@staticmethod | ||
def mock_canonicalize_function(email_msg, canonicalize_as_binary=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we doing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On SENDING, pyas2-lib does not process "canonicalize_as_binary" for the receiving partner, so we need to simulate this. It is not something we want to put our outgoing functionality and also should not put into pyas2-lib. So this is for simulation only.
Without Mock:
b'Content-Type: application/edi-consent\r\nContent-Transfer-Encoding: 7bit\r\nContent-Disposition: attachment; filename="testmessage.edi"\r\n\r\nUNB+UNOA:2+<Sender GLN>:14+<Receiver GLN>:14+140407:0910+5++++1+EANCOM\'\r\nUNH+1+ORDERS:D:96A:UN:EAN008\'\r\nBGM+220+1AA1TEST+9\'\r\nDTM+137:20140407:102\'\r\nDTM+63:20140421:102\'\r\nDTM+64:20140414:102\'\r\nRFF+ADE:1234\'\r\nRFF+PD:1704\'\r\nNAD+BY+5450534000024::9\'\r\nNAD+SU+<Supplier GLN>::9\'\r\nNAD+DP+5450534000109::9+++++++GB\'\r\nNAD+IV+5450534000055::9++AMAZON EU SARL:5 RUE PLAETIS LUXEMBOURG+CO PO BOX 4558+SLOUGH++SL1 0TX+GB\'\r\nRFF+VA:GB727255821\'\r\nCUX+2:EUR:9\'\r\nLIN+1++9783898307529:EN\'\r\nQTY+21:5\'\r\nPRI+AAA:27.5\'\r\nLIN+2++390787706322:UP\'\r\nQTY+21:1\'\r\nPRI+AAA:10.87\'\r\nLIN+3\'\r\nPIA+5+3899408268X-39:SA\'\r\nQTY+21:3\'\r\nPRI+AAA:3.85\'\r\nUNS+S\'\r\nCNT+2:3\'\r\nUNT+26+1\'\r\nUNZ+1+5\'\r\n'
With Mock:
b'Content-Type: application/edi-consent\r\nContent-Transfer-Encoding: 7bit\r\nContent-Disposition: attachment; filename="testmessage.edi"\r\n\r\nUNB+UNOA:2+<Sender GLN>:14+<Receiver GLN>:14+140407:0910+5++++1+EANCOM\'\nUNH+1+ORDERS:D:96A:UN:EAN008\'\nBGM+220+1AA1TEST+9\'\nDTM+137:20140407:102\'\nDTM+63:20140421:102\'\nDTM+64:20140414:102\'\nRFF+ADE:1234\'\nRFF+PD:1704\'\nNAD+BY+5450534000024::9\'\nNAD+SU+<Supplier GLN>::9\'\nNAD+DP+5450534000109::9+++++++GB\'\nNAD+IV+5450534000055::9++AMAZON EU SARL:5 RUE PLAETIS LUXEMBOURG+CO PO BOX 4558+SLOUGH++SL1 0TX+GB\'\nRFF+VA:GB727255821\'\nCUX+2:EUR:9\'\nLIN+1++9783898307529:EN\'\nQTY+21:5\'\nPRI+AAA:27.5\'\nLIN+2++390787706322:UP\'\nQTY+21:1\'\nPRI+AAA:10.87\'\nLIN+3\'\nPIA+5+3899408268X-39:SA\'\nQTY+21:3\'\nPRI+AAA:3.85\'\nUNS+S\'\nCNT+2:3\'\nUNT+26+1\'\nUNZ+1+5\'\n'
mock_canonicalize.side_effect = self.mock_canonicalize_function | ||
|
||
as2message.build( | ||
self.payload, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are using the same payload as the other tests so how is canonicalize_as_binary affecting this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me try to elaborate. The symptoms I had seen, which this functionality resolves was:
linefeeds were not <CR><LF>
for canonicalization but just <LF>
.
When pyas2-lib canonicalizes an input file that only has <LF>
(which our current input file has), it will replace it with <CR><LF>
- the same for the as2message.content function. This is caused by mime_2_bytes.
So in order to mimic that someone basically sends and creates the signature to only have content with just <LF>
, we must prevent that from happening on the outgoing. I had two options to do this:
- I could have made a test providing just the as2message.content as part of the test (basically the result of the mocks) and put that into the test
- imitate the production of such a message using all the functionalities that are there so that when we need to check this further, we better understand all the dependencies.
I chose the later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would choose the first option here because the intent of the test is more clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can change it - not sure I can make it before May though - just fixing the content did not yield the result, so will need more time to dig into it. Making the first option has another downside - if for whatever reason we need to recreate the content, the way how it was done will be lost (unless someone will find this discussion here).
I do question however, if the efforts to make this "round trip" test is worthwhile as the functionality in pyas2-lib was made to patch a specific partner solution that in my opinion behaves out of standard specifications.
message_id=in_message.message_id, direction="IN" | ||
) | ||
|
||
receiver.canonicalize_as_binary = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we doing this here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "receiver"'s canonicalize_as_binary is set to False by default in the setup. I do set it to True to test the functionality during the test and then set it back to the setup value. I probably should persist the initial value of the setup and the reset to that. Will make that change as well in next update.
In relation to #99