Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Markdown autolink resolution _is_mailto can fail with bad string indexing #42139

Closed
tlienart opened this issue Sep 7, 2021 · 0 comments · Fixed by #42140
Closed

Markdown autolink resolution _is_mailto can fail with bad string indexing #42139

tlienart opened this issue Sep 7, 2021 · 0 comments · Fixed by #42140

Comments

@tlienart
Copy link
Contributor

tlienart commented Sep 7, 2021

This was reported by a Franklin user but ended up being a bug with the Markdown module:

julia> Markdown.parse("<一轮红日初升>")
ERROR: StringIndexError: invalid index [6], valid nearby indices [4]=>'', [7]=>''

This is because Markdown sees the < ... > and tries to form an auto link and seems to be using string indexing in doing so. CommonMark.jl handles this well

julia> import CommonMark as CM
julia> p = CM.Parser()
julia> CM.html(p("<一轮红日初升>"))
"<p>&lt;一轮红日初升&gt;</p>\n"

actually reading the full stack trace points to _is_mailto being the culprit

function _is_mailto(s::AbstractString)
length(s) < 6 && return false
# slicing strings is a bit risky, but this equality check is safe
lowercase(s[1:6]) == "mailto:" || return false
return occursin(_email_regex, s[6:end])
end

ironically there's a comment specifying that it's a bit risky to be slicing the string (also it's a bit odd to check the first 6 characters with a 7 character string)

A suggestion would be to replace

lowercase(s[1:6]) == "mailto:" || return false
return occursin(_email_regex, s[6:end])

using first and nextind, I opened a PR with this here: #42140

KristofferC pushed a commit that referenced this issue Sep 9, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant