pleroma-elixir-libraries/bbcode
1
0
Fork 1
Code Issues Pull requests 2 Projects Releases Packages Wiki Activity

WIP: Fix various misparsing/crash issues #10

Draft
phnt wants to merge 5 commits from phnt/bbcode:various-fixes into master
pull from: phnt/bbcode:various-fixes
merge into: pleroma-elixir-libraries:master
Conversation 3 Commits 5 Files changed 3 +253 -18
phnt commented 2026-03-03 21:27:29 +00:00
Member
Copy link
  • Adds more tests for more free-form documents with unknown tags
  • Adds url/img tests to parser
  • Fixes NimbleParsec crash when parsing document with stray [, ]
  • Fixes NimbleParsec crash when parsing document with (unknown) closing-only brackets
  • Fixes NimbleParsec crash when parsing document with a pair of unknown tags
  • Makes end_tag strict and only match on valid tags, replaces lookahead_not(string("[/")) with lookahead_not(end_tag) as well

Superseeds !8

* Adds more tests for more free-form documents with unknown tags * Adds url/img tests to parser * Fixes NimbleParsec crash when parsing document with stray `[, ]` * Fixes NimbleParsec crash when parsing document with (unknown) closing-only brackets * Fixes NimbleParsec crash when parsing document with a pair of unknown tags * Makes `end_tag` strict and only match on valid tags, replaces `lookahead_not(string("[/"))` with `lookahead_not(end_tag)` as well Superseeds !8
Fixes parsing when "rest" only contains one bracket.
phnt commented 2026-03-25 20:27:17 +00:00
Author
Member
Copy link
#cofe

[ul]
[li]asdf[/li]
[/ul]

will result in double newlines after #cofe despite block_tag having ignore(optional(utf8_string([?\n, ?\r], min: 1, max: 2))) , but I have no idea how to fix that.

  1) test nested tags cofe (BBCode.Parser.Test)
     test/bbcode/parser_test.exs:59
     match (=) failed
     code:  assert {:ok, ["#cofe", {:br}, ul: [li: "asdf"]]} = Parser.parse(data)
     left:  {:ok, ["#cofe", {:br}, {:ul, [li: "asdf"]}]}
     right: {:ok, ["#cofe", {:br}, {:br}, {:ul, {:li, "asdf"}}]}
     stacktrace:
       test/bbcode/parser_test.exs:68: (test)

EDIT: Probably not worth trying to fix this. [quote] and probably all the other block tags have the same behavior, it's mostly the line spacing that looks weird in the FE as a result of this behavior in lists.

``` #cofe [ul] [li]asdf[/li] [/ul] ``` will result in double newlines after #cofe despite `block_tag` having `ignore(optional(utf8_string([?\n, ?\r], min: 1, max: 2))) `, but I have no idea how to fix that. ``` 1) test nested tags cofe (BBCode.Parser.Test) test/bbcode/parser_test.exs:59 match (=) failed code: assert {:ok, ["#cofe", {:br}, ul: [li: "asdf"]]} = Parser.parse(data) left: {:ok, ["#cofe", {:br}, {:ul, [li: "asdf"]}]} right: {:ok, ["#cofe", {:br}, {:br}, {:ul, {:li, "asdf"}}]} stacktrace: test/bbcode/parser_test.exs:68: (test) ``` EDIT: Probably not worth trying to fix this. `[quote]` and probably all the other block tags have the same behavior, it's mostly the line spacing that looks weird in the FE as a result of this behavior in lists.
phnt changed title from WIP: Fix various misparsing/crash issues to Fix various misparsing/crash issues 2026-03-25 20:28:46 +00:00
lanodan commented 2026-03-26 12:20:45 +00:00
Owner
Copy link

Looks fine to me, one that would be appreciable is better commit messages like what's listed in the MR description seems a lot better. (git rebase -i allows to do this)

About double-newline after cofe, does it still happens with this kind of source text?

#cofe
[ul]
[li]asdf[/li]
[/ul]
Looks fine to me, one that would be appreciable is better commit messages like what's listed in the MR description seems a lot better. (``git rebase -i`` allows to do this) About double-newline after cofe, does it still happens with this kind of source text? ```bbcode #cofe [ul] [li]asdf[/li] [/ul] ```
phnt commented 2026-03-26 13:40:11 +00:00
Author
Member
Copy link

@lanodan wrote in #10 (comment):

Looks fine to me, one that would be appreciable is better commit messages like what's listed in the MR description seems a lot better. (git rebase -i allows to do this)

Will do.

About double-newline after cofe, does it still happens with this kind of source text?

#cofe
[ul]
[li]asdf[/li]
[/ul]

No, I think it's the newline stanza being matched twice. Once at the end of cofe (text_stanza does not eat the newline - it shouldn't) and the second time on the blank line. Which is the correct behavior, but it looks weird in the FE with a big gap between the two.

image
image

Also I've just noticed that the generator will happily put <li> outside of <ul>/<ol> blocks which is not valid HTML, because the parser doesn't differentiate them.

@lanodan wrote in https://git.pleroma.social/pleroma-elixir-libraries/bbcode/pulls/10#issuecomment-115129: > Looks fine to me, one that would be appreciable is better commit messages like what's listed in the MR description seems a lot better. (`git rebase -i` allows to do this) Will do. > About double-newline after cofe, does it still happens with this kind of source text? > > ```bbcode > #cofe > [ul] > [li]asdf[/li] > [/ul] > ``` No, I think it's the newline stanza being matched twice. Once at the end of cofe (`text_stanza` does not eat the newline - it shouldn't) and the second time on the blank line. Which is the correct behavior, but it looks weird in the FE with a big gap between the two. ![image](/attachments/43cce05e-8bad-49ad-8584-48bc92bab751) ![image](/attachments/304f2323-8024-44ae-b73f-bbbaa291c1a4) Also I've just noticed that the generator will happily put `<li>` outside of `<ul>`/`<ol>` blocks which is not valid HTML, because the parser doesn't differentiate them.
image.png
18 KiB
image.png
18 KiB
phnt changed title from Fix various misparsing/crash issues to WIP: Fix various misparsing/crash issues 2026-03-26 13:40:13 +00:00
phnt force-pushed various-fixes from 13ae3d3b19 to de07dcdf94 2026-03-26 13:40:43 +00:00 Compare
phnt force-pushed various-fixes from de07dcdf94 to d2049b05df 2026-04-05 19:19:23 +00:00 Compare
This pull request is marked as a work in progress.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u various-fixes:phnt-various-fixes
git switch phnt-various-fixes

Merge

Merge the changes and update on Forgejo.
git switch master
git merge --no-ff phnt-various-fixes
git switch phnt-various-fixes
git rebase master
git switch master
git merge --ff-only phnt-various-fixes
git switch phnt-various-fixes
git rebase master
git switch master
git merge --no-ff phnt-various-fixes
git switch master
git merge --squash phnt-various-fixes
git switch master
git merge --ff-only phnt-various-fixes
git switch master
git merge phnt-various-fixes
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
pleroma-elixir-libraries/bbcode!10
No description provided.