Skip to content
Snippets Groups Projects

Add support for RESOLVE and RESOLVE_PTR

Merged yuan requested to merge 0x00A5/arti:impl-resolve into main
2 unresolved threads

Work in progress to support RESOLVE and RESOLVE_PTR as mentioned in issue #33 (closed)

Edited by yuan

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 🤖 Triage Bot 🤖 requested review from @nickm

    requested review from @nickm

  • yuan added 24 commits

    added 24 commits

    Compare with previous version

  • yuan changed the description

    changed the description

  • mentioned in issue torspec#58

  • Nick Mathewson
  • Nick Mathewson
    Nick Mathewson @nickm started a thread on commit 58ed1b33
  • 595 ResolvedVal::NontransientError => {
    596 return Err(Error::InternalError("Received not retriable error.".into()))
    597 }
    598 ResolvedVal::Unrecognized(_, _) => {
    599 return Err(Error::InternalError("Unrecognized resolved value.".into()))
    600 }
    601 // Ignoring ResolvedVal::Hostname
    602 _ => (),
    603 };
    604 }
    605
    606 Ok(addrs)
    607 }
    608
    609 /// Start a reverse DNS lookup by using a RESOLVE cell
    610 pub async fn resolve_ptr(self: Arc<Self>, addr: IpAddr) -> Result<Vec<String>> {
    • This function seems to be mostly duplicated from resolve(). I wonder if we can have them both implemented using a single backend somehow, and whether that would save code or not.

    • Author Contributor

      I moved these code around and couldn't come up with a cleaner way than creating a helper function to send the message and read from resolve stream.

      If I combine the two dispatching match block and make the function return a Vec<ResolvedVal> instead, I will need to check again whether it is a hostname or an IP address at the client level. Please let me know if you have better idea to refactor this, I'd like to make the change. Thanks!

      Edited by yuan
    • Please register or sign in to reply
  • Nick Mathewson
  • Nick Mathewson
  • Great patch!

    One question I have here is about testing strategy. I know that some of the stream/circuit code in tor-proto is hard to test right now, but maybe we can refactor some of the message parsing/handling code so that we could test that correctly.

    I've also had a look at the code and left a few comments in various parts. Please feel free to fix anything you want to, and I'll take on the rest once you're done. Thanks so much for the code!

  • yuan added 15 commits

    added 15 commits

    Compare with previous version

  • yuan marked this merge request as ready

    marked this merge request as ready

  • yuan added 11 commits

    added 11 commits

    Compare with previous version

  • Please register or sign in to reply
    Loading