feat: port register command to Rust #1222
Conversation
bbangert
force-pushed
the
feat/issue-1190
branch
2 times, most recently
from
e35e230 to
715f2e5
Compare
Codecov Report
@@ Coverage Diff @@
## master #1222 +/- ##
======================================
Coverage 100% 100%
======================================
Files 60 60
Lines 10197 10190 -7
======================================
- Hits 10197 10190 -7
Continue to review full report at Codecov.
|
| @@ -733,17 +733,15 @@ where | |||
| } | |||
| } | |||
| Either::A(ClientMessage::Register { channel_id, key }) => { | |||
| data.srv.metrics.incr("ua.command.register").ok(); | |||
There was a problem hiding this comment.
there was also an associated log.info here in python
There was a problem hiding this comment.
Yup, we never used it and I don't know why we bothered logging it.
autopush_rs/src/util/ddb_helpers.rs
Outdated
| message_table_name: &str, | ||
| ) -> MyFuture<UpdateItemOutput> { | ||
| let chid = channel_id.hyphenated().to_string(); | ||
| let expiry = (time::get_time().sec as u64) + MAX_EXPIRY; |
autopush_rs/src/server/mod.rs
Outdated
| let key_digest = hash::hash(hash::MessageDigest::sha256(), &raw_key) | ||
| .chain_err(|| "Error creating message digest for key")?; | ||
| base.extend(key_digest.iter()); | ||
| let encrypted = self.opts.fernet.encrypt(&base); |
There was a problem hiding this comment.
let's trim_matches('=') the result of fernet.encrypt here and below (like the python code), just to be super safe
There was a problem hiding this comment.
The Python one only strips the encrypted value once.
There was a problem hiding this comment.
I meant below as in the else block
|
|
||
| def process(self, command): | ||
| # type: (Register) -> Union[RegisterResponse, RegisterErrorResponse] | ||
| valid, msg = _validate_chid(command.channel_id) |
There was a problem hiding this comment.
We have _validate_chid here to mimick websocket.py's validation of the channel_id sent in register (that it's hyphenated).
Now I'm wondering if this check is bogus: because the rust side parses the register command now. It's deserializing it into ClientMessage::Register channel_id: Uuid. Does serde's deserialization allow non hyphenated uuids to pass through unnoticed?
There was a problem hiding this comment.
yea, it does pass it through: https://docs.rs/uuid/0.6.3/src/uuid/serde_support.rs.html#16-56
parse() is parse_str(): "Parses a Uuid from a string of hexadecimal digits with optional hyphens."
There was a problem hiding this comment.
Yes, serde will allow hypenated or not to pass through.
bbangert
force-pushed
the
feat/issue-1190
branch
2 times, most recently
from
be1ef64 to
2d1f97b
Compare
Closes #1190