fix: URL-encode node names in web UI links and redirects (#396, #414)#447
Open
makaiver wants to merge 2 commits into
Open
fix: URL-encode node names in web UI links and redirects (#396, #414)#447makaiver wants to merge 2 commits into
makaiver wants to merge 2 commits into
Conversation
The version-comparison feature built query strings by raw string interpolation of the node name. When a node name contains spaces or other special characters (e.g. "router1 [192.168.1.1]"), selecting a second version POSTs to /node/version/diffs, whose handler redirected with an unencoded Location header. The raw space corrupted the query, the follow-up GET received a truncated node, and the UI showed "No differences available". The first comparison worked only because a browser auto-encodes spaces in an <a href>. Add a url_param helper (Rack::Utils.escape) and use it at the three sites that interpolate node/group/oid into a diff URL: the POST redirect in webapp.rb and the query strings built in versions.haml and diffs.haml. Output is byte-identical for names without special characters, so existing behaviour and tests are unchanged. Fixes ytti#396 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…#414) The ?node_full= links in nodes.haml, node.haml, diffs.haml and version.haml interpolated the node name (full_name) unencoded. A "+" in the name is decoded to a space in a query string, so opening the versions page for a node like "router1+(test123+)" failed. Encode those links with the same url_param helper. The /node/fetch and /node/next PATH links are intentionally left unencoded: they rely on the literal "/" group separator, and "+" is not space-decoded in a path. Fixes ytti#414 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
makaiver
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-Request Checklist
rubocop --auto-correct)rake test)Description
Two related bugs with the same root cause: node names are interpolated into web
UI URLs without URL-encoding, so nodes whose names contain special characters
produce broken links.
#396 — spaces break version comparison
For a node like
router1 [192.168.1.1], the first diff works but selecting asecond version to compare shows "No differences available." The
POST /node/version/diffshandler redirected with an unencodedLocation:header; the raw space corrupted the query, the follow-up
GETgot a truncatednode, and no diff was found. (The first comparison only worked because abrowser auto-encodes spaces in an
<a href>.)#414 —
+breaks the versions pageFor a node like
router1+(test123+), opening/node/version?node_full=…fails,because a
+in a query string is decoded to a space server-side. The?node_full=links were built by raw interpolation offull_name.Fix
Add a small
url_paramhelper (Rack::Utils.escape) and apply it everywhere anode name is interpolated into a URL:
POST /node/version/diffsredirect (webapp.rb) — The version comparison doesn't work correctly. #396diffs.haml,versions.haml) — The version comparison doesn't work correctly. #396?node_full=links (nodes.haml,node.haml,diffs.haml,version.haml) — Plus (+) in hostnames causes non working URLs #414Output is byte-identical for names without special characters, so existing
behaviour and tests are unchanged.
The
/node/fetch/…and/node/next/…path links are intentionally left as-is:they rely on the literal
/group separator (encoding it would break routing),and
+is not space-decoded in a path segment.Tests
post /node/version/diffsasserts the redirectLocationis encoded forrouter1 [192.168.1.1](The version comparison doesn't work correctly. #396).get /nodesasserts the?node_full=link is encoded forrouter1+(test123+)(Plus (+) in hostnames causes non working URLs #414).Closes #396
Closes #414