Skip to content
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

Fix detach&attach not to create new COSEMessage instance #447

Merged
merged 9 commits into from Oct 17, 2023

Conversation

kentakayama
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (753d090) 97.24% compared to head (659da4b) 97.24%.
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #447   +/-   ##
=======================================
  Coverage   97.24%   97.24%           
=======================================
  Files          32       32           
  Lines        3261     3262    +1     
=======================================
+ Hits         3171     3172    +1     
  Misses         90       90           
Files Coverage Δ
cwt/cose_message.py 96.17% <100.00%> (+0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kentakayama kentakayama marked this pull request as draft October 16, 2023 13:21
@kentakayama
Copy link
Collaborator Author

kentakayama commented Oct 16, 2023

I noticed that dumps() method includes the payload even after detach_payload() is done.
Let me give some time to fix it.

cwt/cose_message.py Outdated Show resolved Hide resolved
cwt/cose_message.py Outdated Show resolved Hide resolved
cwt/cose_message.py Outdated Show resolved Hide resolved
@dajiaji dajiaji self-requested a review October 17, 2023 03:46
@kentakayama
Copy link
Collaborator Author

kentakayama commented Oct 17, 2023

@dajiaji
Thank you for your review, now it works.
I have to note that

COSEMessage.loads() on detached content message works fine

This code below prints like this, and it outputs what I expected.

18([<< {1: -7, 3: 0} >>, {4: << -18, -18 >>}, null, h'6520BBAF2081D7E0ED0F95F76EB0733D667005F7467CEC4B87B9381A6BA1EDE8E00DF29F32A37230F39A842A54821FDD223092819D7728EFB9D3A0080B75380B'])

import sys
import subprocess
from cwt import COSEMessage
"""
18([
  / protected: / h'A201260300',
  / unprotected: / {4: h'3131'},
  / payload: / null / detached /,
  / signature: / h'6520BBAF2081D7E0ED0F95F76EB0733D667005F7467CEC4B87B9381A6BA1EDE8E00DF29F32A37230F39A842A54821FDD223092819D7728EFB9D3A0080B75380B'
])
"""
expected_detached_cose_message = COSEMessage.loads(
    bytes.fromhex(
        "D28445A201260300A104423131F658406520BBAF2081D7E0ED0F95F76EB0733D667005F7467CEC4B87B9381A6BA1EDE8E00DF29F32A37230F39A842A54821FDD223092819D7728EFB9D3A0080B75380B"
    )
)
output = subprocess.check_output(f"echo {expected_detached_cose_message.dumps().hex().upper()} | pretty2diag.rb -e", shell=True).decode(sys.stdout.encoding).rstrip("\n")
print(output)

on the other hand,

I expect that countersign on this instance would produce we don't expect

because both self._payload and self._msg[2] is None .
Could you fix it in the future?

@kentakayama kentakayama marked this pull request as ready for review October 17, 2023 03:52
@dajiaji
Copy link
Owner

dajiaji commented Oct 17, 2023

Could you fix it in the future?

Yes, I understand this negative side-effect. I can fix it on my next PR.

@dajiaji dajiaji merged commit 4b00d4e into dajiaji:main Oct 17, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants