mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
fix: only tag explicit http(s) URLs as r references
Posts were publishing a flood of bogus `r` tags — the v1.13.0 release note shipped 11 junk references such as `https://.deb/`, `https://window.nostr/`, `https://kind:30166/` and `https://crowdin.pretended462/`. The reference extractor `findURLs` fed the raw, scheme-less UrlDetector output straight into `r` tags. That detector is deliberately eager: to let the rich-text renderer linkify a bare `example.com`, it also reports every `word.word`, `word/word` or `word:port` token with no real-TLD whitelist. Prose is full of those (`.deb`, `.rpm`, `[database].backend`, `nostr-wallet-connect/nwc`, `~2.5x`, `@mentions`), so each one became a reference on the published note. The rich-text side already guards against this via UrlParser (TLD validation + scheme separation); the tag path never got the same guard. Require an explicit http/https scheme and a valid TLD before a detected URL becomes a reference. Rendering is untouched (separate parser), so bare domains still show as links — they just no longer pollute the tags. Adds regression coverage over the exact fragments from the v1.13.0 note plus checks that real, explicitly-schemed links are still extracted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8KXx8UQ6mgyxjBSWW6sQV
This commit is contained in:
@@ -22,23 +22,60 @@ package com.vitorpamplona.quartz.nip10Notes.content
|
||||
|
||||
import com.vitorpamplona.quartz.utils.DualCase
|
||||
import com.vitorpamplona.quartz.utils.startsWithAny
|
||||
import com.vitorpamplona.quartz.utils.urldetector.Url
|
||||
import com.vitorpamplona.quartz.utils.urldetector.detection.UrlDetector
|
||||
|
||||
val rejectSchemes =
|
||||
/**
|
||||
* Only URLs the author wrote with one of these explicit web schemes become `r`
|
||||
* reference tags. See [findURLs] for why a scheme is required.
|
||||
*/
|
||||
val webSchemes =
|
||||
listOf(
|
||||
DualCase("ftp:"),
|
||||
DualCase("ftps:"),
|
||||
DualCase("ws:"),
|
||||
DualCase("wss:"),
|
||||
DualCase("nostr:"),
|
||||
DualCase("blossom:"),
|
||||
DualCase("http://"),
|
||||
DualCase("https://"),
|
||||
)
|
||||
|
||||
fun findURLs(text: String) =
|
||||
UrlDetector(text).detect().mapNotNull {
|
||||
if (it.originalUrl.startsWithAny(rejectSchemes)) {
|
||||
null
|
||||
/**
|
||||
* True when the host's top-level domain begins with an ASCII letter.
|
||||
*
|
||||
* ICANN does not allow numeric-only TLDs, so a "host" whose TLD starts with a
|
||||
* digit — the `2.5x` in `~2.5x`, for instance — is prose, not a real domain.
|
||||
* IPv6 literal hosts are bracketed (`[2001:db8::1]`) and have no dotted TLD, so
|
||||
* accept them directly. Mirrors `UrlParser.isValidTopLevelDomain` on the
|
||||
* rich-text side.
|
||||
*/
|
||||
private fun Url.hasValidTopLevelDomain(): Boolean {
|
||||
if (host.startsWith('[')) return true
|
||||
val startOfTld = host.lastIndexOf('.') + 1
|
||||
if (startOfTld >= host.length) return false
|
||||
val first = host[startOfTld]
|
||||
return first in 'a'..'z' || first in 'A'..'Z'
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the http(s) URLs mentioned in [text], for building `r` reference tags.
|
||||
*
|
||||
* The underlying [UrlDetector] is deliberately eager: to help the rich-text
|
||||
* renderer linkify a bare `example.com`, it also reports scheme-less "domains" —
|
||||
* any `word.word`, `word/word`, or `word:port` token, with no real-TLD whitelist.
|
||||
* That is far too loose for reference tags. Prose is full of such tokens
|
||||
* (`.deb`, `.rpm`, `window.nostr`, `kind:30166`, `[database].backend`,
|
||||
* `nostr-wallet-connect/nwc`, `crowdin.pretended462`, `~2.5x`, `@mentions`), and
|
||||
* every one of them used to become a bogus `r` tag on the published note.
|
||||
*
|
||||
* So a token qualifies as a reference only when the author actually wrote it with
|
||||
* an explicit http/https scheme and it carries a valid TLD. The renderer keeps
|
||||
* its own, looser parser ([com.vitorpamplona.amethyst.commons.richtext.UrlParser]),
|
||||
* so bare domains are still shown as links — they just no longer pollute the tags.
|
||||
*/
|
||||
fun findURLs(text: String): List<String> =
|
||||
UrlDetector(text).detect().mapNotNull { url ->
|
||||
if (url.urlMarker.hasScheme() &&
|
||||
url.originalUrl.startsWithAny(webSchemes) &&
|
||||
url.hasValidTopLevelDomain()
|
||||
) {
|
||||
url.originalUrl
|
||||
} else {
|
||||
it.originalUrl
|
||||
null
|
||||
}
|
||||
}
|
||||
|
||||
+83
@@ -0,0 +1,83 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.quartz.nip10Notes.urls
|
||||
|
||||
import com.vitorpamplona.quartz.nip10Notes.content.findURLs
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertContains
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* Regression tests for the flood of bogus `r` tags on published notes (see the
|
||||
* Amethyst v1.13.0 release note, which shipped 11 junk references such as
|
||||
* `https://.deb/`, `https://window.nostr/`, `https://kind:30166/` and
|
||||
* `https://crowdin.pretended462/`).
|
||||
*
|
||||
* Every one of those came from a scheme-less prose token that the eager
|
||||
* [findURLs] used to accept. It must now only return URLs the author wrote with
|
||||
* an explicit http/https scheme.
|
||||
*/
|
||||
class UrlsFalsePositiveRepro {
|
||||
/** The exact prose fragments from the v1.13.0 note that produced junk `r` tags. */
|
||||
@Test
|
||||
fun schemelessProseTokensAreNotReferences() {
|
||||
val fragments =
|
||||
listOf(
|
||||
"`.deb`/`.rpm` packages with a bundled JRE",
|
||||
"German by crowdin.pretended462",
|
||||
"dead-relay cache backed by kind:30166 events",
|
||||
"backend via `[database].backend`",
|
||||
"NIP-07 `window.nostr` provider",
|
||||
"Namecoin `.bit`",
|
||||
"using the new `pay`/`receive` methods\n (nostr-wallet-connect/nwc#2)",
|
||||
"a rich composer (@mentions, custom-emoji autocomplete",
|
||||
"direct-built wire frames (~2.5x)",
|
||||
"with `serve`/`up`",
|
||||
)
|
||||
|
||||
for (f in fragments) {
|
||||
assertEquals(emptyList(), findURLs(f), "Expected no references in: $f")
|
||||
}
|
||||
}
|
||||
|
||||
/** Real, explicitly-schemed URLs are still detected. */
|
||||
@Test
|
||||
fun explicitHttpUrlsAreStillReferences() {
|
||||
assertContains(findURLs("read https://example.com/a/b now"), "https://example.com/a/b")
|
||||
assertContains(findURLs("see http://plan9.bell-labs.com"), "http://plan9.bell-labs.com")
|
||||
|
||||
val two = findURLs("I have a website at https://mysite.xyz and a blog at https://myblog.xyz")
|
||||
assertContains(two, "https://mysite.xyz")
|
||||
assertContains(two, "https://myblog.xyz")
|
||||
assertEquals(2, two.size)
|
||||
}
|
||||
|
||||
/** A real link embedded in the middle of the junk-heavy note is still recovered. */
|
||||
@Test
|
||||
fun realLinkSurvivesAmongProse() {
|
||||
val text = "Grab the `.deb` from https://github.com/vitorpamplona/amethyst/releases and window.nostr does the rest"
|
||||
val urls = findURLs(text)
|
||||
assertContains(urls, "https://github.com/vitorpamplona/amethyst/releases")
|
||||
assertTrue(urls.none { it.contains("window.nostr") }, "window.nostr must not be a reference: $urls")
|
||||
assertTrue(urls.none { it.contains(".deb") }, ".deb must not be a reference: $urls")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user