Skip to content

Commit

Permalink
Fix isAllowed of escapeStreamer (#22814)
Browse files Browse the repository at this point in the history
The use of `sort.Search` is wrong: The slice should be sorted, and
`return >= 0` doen't mean it exists, see the
[manual](https://pkg.go.dev/sort#Search).

Could be fixed like this if we really need it:

```diff
diff --git a/modules/charset/escape_stream.go b/modules/charset/escape_stream.go
index 823b63513..fcf1ffbc1 100644
--- a/modules/charset/escape_stream.go
+++ b/modules/charset/escape_stream.go
@@ -20,6 +20,9 @@ import (
 var defaultWordRegexp = regexp.MustCompile(`(-?\d*\.\d\w*)|([^\` + "`" + `\~\!\@\#\$\%\^\&\*\(\)\-\=\+\[\{\]\}\\\|\;\:\'\"\,\.\<\>\/\?\s\x00-\x1f]+)`)

 func NewEscapeStreamer(locale translation.Locale, next HTMLStreamer, allowed ...rune) HTMLStreamer {
+       sort.Slice(allowed, func(i, j int) bool {
+               return allowed[i] < allowed[j]
+       })
        return &escapeStreamer{
                escaped:                 &EscapeStatus{},
                PassthroughHTMLStreamer: *NewPassthroughStreamer(next),
@@ -284,14 +287,8 @@ func (e *escapeStreamer) runeTypes(runes ...rune) (types []runeType, confusables
 }

 func (e *escapeStreamer) isAllowed(r rune) bool {
-       if len(e.allowed) == 0 {
-               return false
-       }
-       if len(e.allowed) == 1 {
-               return e.allowed[0] == r
-       }
-
-       return sort.Search(len(e.allowed), func(i int) bool {
+       i := sort.Search(len(e.allowed), func(i int) bool {
                return e.allowed[i] >= r
-       }) >= 0
+       })
+       return i < len(e.allowed) && e.allowed[i] == r
 }
```

But I don't think so, a map is better to do it.
  • Loading branch information
wolfogre authored Feb 9, 2023
1 parent 29aea36 commit e253888
Showing 1 changed file with 7 additions and 17 deletions.
24 changes: 7 additions & 17 deletions modules/charset/escape_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package charset
import (
"fmt"
"regexp"
"sort"
"strings"
"unicode"
"unicode/utf8"
Expand All @@ -20,12 +19,16 @@ import (
var defaultWordRegexp = regexp.MustCompile(`(-?\d*\.\d\w*)|([^\` + "`" + `\~\!\@\#\$\%\^\&\*\(\)\-\=\+\[\{\]\}\\\|\;\:\'\"\,\.\<\>\/\?\s\x00-\x1f]+)`)

func NewEscapeStreamer(locale translation.Locale, next HTMLStreamer, allowed ...rune) HTMLStreamer {
allowedM := make(map[rune]bool, len(allowed))
for _, v := range allowed {
allowedM[v] = true
}
return &escapeStreamer{
escaped: &EscapeStatus{},
PassthroughHTMLStreamer: *NewPassthroughStreamer(next),
locale: locale,
ambiguousTables: AmbiguousTablesForLocale(locale),
allowed: allowed,
allowed: allowedM,
}
}

Expand All @@ -34,7 +37,7 @@ type escapeStreamer struct {
escaped *EscapeStatus
locale translation.Locale
ambiguousTables []*AmbiguousTable
allowed []rune
allowed map[rune]bool
}

func (e *escapeStreamer) EscapeStatus() *EscapeStatus {
Expand Down Expand Up @@ -256,7 +259,7 @@ func (e *escapeStreamer) runeTypes(runes ...rune) (types []runeType, confusables
runeCounts.numBrokenRunes++
case r == ' ' || r == '\t' || r == '\n':
runeCounts.numBasicRunes++
case e.isAllowed(r):
case e.allowed[r]:
if r > 0x7e || r < 0x20 {
types[i] = nonBasicASCIIRuneType
runeCounts.numNonConfusingNonBasicRunes++
Expand All @@ -282,16 +285,3 @@ func (e *escapeStreamer) runeTypes(runes ...rune) (types []runeType, confusables
}
return types, confusables, runeCounts
}

func (e *escapeStreamer) isAllowed(r rune) bool {
if len(e.allowed) == 0 {
return false
}
if len(e.allowed) == 1 {
return e.allowed[0] == r
}

return sort.Search(len(e.allowed), func(i int) bool {
return e.allowed[i] >= r
}) >= 0
}

0 comments on commit e253888

Please sign in to comment.