<p></p>
<p dir="auto">I think the underlying problem here is that we are using the keys in our urls, without any validation to ensure that they are url safe. A similar example would be our validation of user display_names, which are validated as url safe, since we use those in urls.</p>
<div class="highlight highlight-source-diff" dir="auto"><pre> validates :user, :associated => true
<span class="pl-md"><span class="pl-md">-</span> validates :k, :v, :length => 1..255, :characters => true</span>
<span class="pl-mi1"><span class="pl-mi1">+</span> validates :k, :length => 1..255, :characters => { :url_safe => true }</span>
<span class="pl-mi1"><span class="pl-mi1">+</span> validates :v, :length => 1..255, :characters => true</span>
end</pre></div>
<p dir="auto">It's a bit surprising that this hasn't come up until now! And it's quite funny to me that the problem is highlighted by the keys that we use internally for gpx visibility and diary default language, both of which use "." characters <g-emoji class="g-emoji" alias="smile" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f604.png">😄</g-emoji></p>
<p dir="auto">Unfortunately I'm not in favour of the proposed solution. We've been trying to move away from plain-text API responses, mostly for consistency reasons between API endpoints but also for developer ergonomics (you don't want to have some responses handled by an xml or json library, but have to handle some responses that aren't). But we have also settled on the convention that you can choose between response formats using extensions e.g. <code class="notranslate">GET /api/0.6/node/1.json</code>. If we merged this PR, we would not be able to do <code class="notranslate">GET /api/.../user/preferences/key.json</code> in future.</p>
<p dir="auto">I'm open to further discussion and any suggestions!</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/3787#issuecomment-1309387342">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLNRDTFURCOZKFOVKADWHQIOXANCNFSM6AAAAAARZPOMC4">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLJD3IJWM3TQIAU7RIDWHQIOXA5CNFSM6AAAAAARZPOMC6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTSOBOVE4.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/3787/c1309387342</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/3787#issuecomment-1309387342",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/3787#issuecomment-1309387342",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>