From f67a251e275727cfb599237a9ba0d1af9806cf47 Mon Sep 17 00:00:00 2001 From: Niklas Fasching Date: Sat, 24 Aug 2019 12:11:23 +0200 Subject: [PATCH] html: Fix html writer footnotes (in headlines) writer.footnotes must be a pointer as we copy the writer in nodesAsString() and can thus end up modifying the footnotes.list slice without it being reflected in the original writer (i.e. when the backing array of the slice changes). --- org/html_writer.go | 4 +- org/testdata/footnotes_in_headline.html | 47 +++++++++++++++++++ org/testdata/footnotes_in_headline.org | 10 ++++ org/testdata/footnotes_in_headline.pretty_org | 10 ++++ 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 org/testdata/footnotes_in_headline.html create mode 100644 org/testdata/footnotes_in_headline.org create mode 100644 org/testdata/footnotes_in_headline.pretty_org diff --git a/org/html_writer.go b/org/html_writer.go index ad1bedd..6355fd1 100644 --- a/org/html_writer.go +++ b/org/html_writer.go @@ -19,7 +19,7 @@ type HTMLWriter struct { HighlightCodeBlock func(source, lang string) string htmlEscape bool log *log.Logger - footnotes footnotes + footnotes *footnotes } type footnotes struct { @@ -61,7 +61,7 @@ func NewHTMLWriter() *HTMLWriter { HighlightCodeBlock: func(source, lang string) string { return fmt.Sprintf("
\n
\n%s\n
\n
", html.EscapeString(source)) }, - footnotes: footnotes{ + footnotes: &footnotes{ mapping: map[string]int{}, }, } diff --git a/org/testdata/footnotes_in_headline.html b/org/testdata/footnotes_in_headline.html new file mode 100644 index 0000000..489169a --- /dev/null +++ b/org/testdata/footnotes_in_headline.html @@ -0,0 +1,47 @@ + +

+Title 1 +

+
+
+
+
+1 +
+

+this test file just exists to reproduce a bug with footnotes in headlines - that only happens in very specific circumstances. +The TLDR is: +

+
    +
  • +

    +HTMLWriter.footnotes should be a pointer field. I didn't notice my error as go translated my pointer-method calls on +non-pointer values rather than complaining - i.e. footnotes.add() transparently gets translated to (&footnotes).add() (docs). +

    +
    • +
  • +

    +Headlines have to be htmlified twice - once for the outline and once for the headline itself. To do so we have to copy the writer +

    +
    • +
  • +

    +Copying the writer copies footnotes - which contains a map and a slice. Changes to the map will always be reflected in the original map. +Changes to the slice will only be reflected if the slice doesn't grow. +

    +
    • +
  • +

    +We can thus end up with a footnote being in the mapping but not the slice - and get an index out of range error. +

    +
    • +
+
+
+
+
diff --git a/org/testdata/footnotes_in_headline.org b/org/testdata/footnotes_in_headline.org new file mode 100644 index 0000000..3d9886c --- /dev/null +++ b/org/testdata/footnotes_in_headline.org @@ -0,0 +1,10 @@ +* Title [fn:1] + +[fn:1] this test file just exists to reproduce a bug with footnotes in headlines - that only happens in very specific circumstances. +The TLDR is: +- HTMLWriter.footnotes should be a pointer field. I didn't notice my error as go translated my pointer-method calls on + non-pointer values rather than complaining - i.e. =footnotes.add()= transparently gets translated to =(&footnotes).add()= ([[https://golang.org/ref/spec#Calls][docs]]). +- Headlines have to be htmlified twice - once for the outline and once for the headline itself. To do so we have to copy the writer +- Copying the writer copies footnotes - which contains a map and a slice. Changes to the map will always be reflected in the original map. + Changes to the slice will only be reflected if the slice doesn't grow. +- We can thus end up with a footnote being in the mapping but not the slice - and get an index out of range error. diff --git a/org/testdata/footnotes_in_headline.pretty_org b/org/testdata/footnotes_in_headline.pretty_org new file mode 100644 index 0000000..3d9886c --- /dev/null +++ b/org/testdata/footnotes_in_headline.pretty_org @@ -0,0 +1,10 @@ +* Title [fn:1] + +[fn:1] this test file just exists to reproduce a bug with footnotes in headlines - that only happens in very specific circumstances. +The TLDR is: +- HTMLWriter.footnotes should be a pointer field. I didn't notice my error as go translated my pointer-method calls on + non-pointer values rather than complaining - i.e. =footnotes.add()= transparently gets translated to =(&footnotes).add()= ([[https://golang.org/ref/spec#Calls][docs]]). +- Headlines have to be htmlified twice - once for the outline and once for the headline itself. To do so we have to copy the writer +- Copying the writer copies footnotes - which contains a map and a slice. Changes to the map will always be reflected in the original map. + Changes to the slice will only be reflected if the slice doesn't grow. +- We can thus end up with a footnote being in the mapping but not the slice - and get an index out of range error.