Skip to content

Make domain names str instead of bytes throughout the code, and only convert to bytes on serialization#1027

Merged
KPrasch merged 1 commit intonucypher:hawksbeardfrom
fjarri:proper-decode-for-domain-names
May 30, 2019
Merged

Make domain names str instead of bytes throughout the code, and only convert to bytes on serialization#1027
KPrasch merged 1 commit intonucypher:hawksbeardfrom
fjarri:proper-decode-for-domain-names

Conversation

@fjarri
Copy link
Contributor

@fjarri fjarri commented May 29, 2019

Fixes #1026

@fjarri fjarri force-pushed the proper-decode-for-domain-names branch from cf376a2 to 4e6782a Compare May 30, 2019 10:40
@codecov
Copy link

codecov bot commented May 30, 2019 •
edited
Loading

Codecov Report

Merging #1027 into hawksbeard will decrease coverage by 3.99%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           hawksbeard    #1027    +/-   ##
============================================
- Coverage        81.1%   77.11%    -4%     
============================================
  Files              67       67            
  Lines            8560     8559     -1     
============================================
- Hits             6943     6600   -343     
- Misses           1617     1959   +342
Impacted Files Coverage Δ
nucypher/cli/characters/ursula.py 67.44% <100%> (-0.94%) ⬇️
nucypher/cli/characters/felix.py 68% <100%> (-1%) ⬇️
nucypher/characters/lawful.py 76.66% <100%> (-11.58%) ⬇️
nucypher/config/node.py 80.11% <100%> (-2.69%) ⬇️
nucypher/utilities/sandbox/constants.py 98.11% <100%> (ø) ⬆️
nucypher/characters/control/interfaces.py 36.53% <0%> (-53.85%) ⬇️
nucypher/characters/control/controllers.py 40.67% <0%> (-43.23%) ⬇️
nucypher/characters/control/serializers.py 43.44% <0%> (-37.71%) ⬇️
nucypher/characters/control/specifications.py 56.6% <0%> (-35.85%) ⬇️
nucypher/cli/characters/alice.py 35.41% <0%> (-28.13%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae7edee...a9622b8. Read the comment docs.

@fjarri fjarri force-pushed the proper-decode-for-domain-names branch 2 times, most recently from 0022ab3 to 3175cb0 Compare May 30, 2019 11:02
@fjarri fjarri force-pushed the proper-decode-for-domain-names branch from 3175cb0 to a9622b8 Compare May 30, 2019 11:25
@fjarri fjarri changed the title Decode domain names before serialization to JSON and printing Make domain names str instead of bytes throughout the code, and only convert to bytes on serialization May 30, 2019
@fjarri fjarri requested a review from KPrasch May 30, 2019 11:36
@fjarri fjarri marked this pull request as ready for review May 30, 2019 11:36
KPrasch
KPrasch approved these changes May 30, 2019
View reviewed changes
Copy link
Member

@KPrasch KPrasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤠

KPrasch
KPrasch reviewed May 30, 2019
View reviewed changes
nucypher/cli/characters/ursula.py
else:

# Domains -> bytes | or default
domains = set(bytes(network, encoding='utf-8')) if network else None
Copy link
Member

@KPrasch KPrasch May 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow - this was a horrendous bug. Nice Find!

@KPrasch KPrasch merged commit 2a5504a into nucypher:hawksbeard May 30, 2019
@fjarri fjarri deleted the proper-decode-for-domain-names branch May 30, 2019 13:43
This was referenced May 31, 2019
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KPrasch KPrasch KPrasch approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fjarri @KPrasch