#6946: Allow filtering relays by OS -----------------------------+-------------------------------- Reporter: rransom | Owner: irl Type: enhancement | Status: needs_revision Priority: Medium | Milestone: Component: Metrics/Onionoo | Version: Severity: Normal | Resolution: Keywords: | Actual Points: Parent ID: | Points: Reviewer: karsten | Sponsor: -----------------------------+--------------------------------
Comment (by irl): Replying to [comment:15 karsten]: > Some things I noticed: > - You're reusing `ResourceServlet#parseVersionParameter` for parsing the `os` parameter. Wouldn't it make more sense to reuse `#parseContactParameter` here? After all, we'll want to support searches for `"Linux"` as well as for `"Windows 95"` here, right? Also, maybe rename the method to `#parseContactOrOsParameter` to make it clear that the method is used for both. I've added a new `#parseOperatingSystemParameter` in a fixup commit. When we're looking at UTF-8 for [[https://gitweb.torproject.org/torspec.git/tree/proposals/285-utf-8.txt|proposal 285]] it would be really good to refactor all the `#parseXParameter` functions into what they are actually doing, e.g. only printable UTF-8 instead of what the parameter name is. I could not use `#parseContactParameter` as this was splitting on spaces. > - Did you try out what happens when you search for strings that contain spaces? And did you also try out using the new `"os"` parameter in a qualified search term? IIRC, we did not fix the space issue in qualified search terms, and in this case it may bite us again. I don't have an easy fix for this. I've checked now that spaces do work. In the search string though, we can only search as far as the first space. We can ensure we don't hit any errors in Relay Search by never trying to build search strings that have spaces in the OS. It may not be the best solution but for now at least we do increase functionality, even if it's not to where it could be. > - Do you mind extending `ResourceServletTest` to actually test different requests using the new parameter? Maybe the `#testContactXY` methods serve as a template. Ok. Will try to do this this afternoon or tomorrow morning. That looks easy enough. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/6946#comment:16> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs