Memos highlight sync fails with error in decode.lua

[landure]

• KOReader version: v2024.04
• Device: Kobo Touch

Issue

When syncing highlight to Memos, KOReader crash. See crashlog.

Steps to reproduce

The book metadata file is metadata.epub.lua.gz.

It should contain the value causing the decoding issue.

crash.log (if applicable)

crash.log

06/12/24-22:12:36 DEBUG _refresh: Enqueued flashui update for region 0 0 600 800 dithering: false 
06/12/24-22:12:36 DEBUG triggering refresh {
  mode = "flashui",
  region = "600x800+0+0"
} --[[table: 0x2eb610a8]] 
./luajit: rocks/share/lua/5.1/json/decode.lua:91: Unclosed elements present
stack traceback:
	[C]: in function 'assert'
	rocks/share/lua/5.1/json/decode.lua:91: in function 'decode'
	plugins/exporter.koplugin/target/memos.lua:41: in function 'makeRequest'
	plugins/exporter.koplugin/target/memos.lua:139: in function 'createHighlights'
	plugins/exporter.koplugin/target/memos.lua:156: in function 'export'
	plugins/exporter.koplugin/main.lua:206: in function 'action'
	frontend/ui/uimanager.lua:1012: in function '_checkTasks'
	frontend/ui/uimanager.lua:1460: in function 'handleInput'
	frontend/ui/uimanager.lua:1580: in function 'run'
	./reader.lua:280: in main chunk
	[C]: at 0x000140cd
!!!!
Uh oh, something went awry... (Crash n°1: 06/12/24 @ 22:12:39)
Running FW 4.38.21908 on Linux 2.6.35.3-850-gbc67621+ (#619 PREEMPT Thu Dec 22 15:29:00 CST 2016)
Attempting to restart KOReader . . .
!!!!
---------------------------------------------
                launching...

[pazos]

The book metadata file is metadata.epub.lua.gz.

It should contain the value causing the decoding issue.

I'm afraid it doesn't work that way.
We encode json to send data to the server. We decode json server responses.

The invalid decoding happens in https://github.com/koreader/koreader/blob/master/plugins/exporter.koplugin/target/memos.lua#L41

./luajit: rocks/share/lua/5.1/json/decode.lua:91: Unclosed elements present

That tell us the server response wasn't valid.

The crash is entirely our fault. If the json is malformed we should return false.

@fierceX: could you please look in to the issue?. If memos server can reply invalid json I think we should put json.decode in a protected call.

[landure]

I used mitmproxy to capture the queries issued by KOReader. Only one query is made before the crash occurs.
According to https://jsonlint.com/, the response JSON is valid.

Here is the query made by KOReader to the Memos instance:

{"content":"Debugging Checklist\nIs the problem being reported a direct result of the underlying bug, or merely a symptom?\nIs the bug really in the framework you’re using? Is it in the OS? Or is it in your code?\nIf you explained this problem in detail to a coworker, what would you say?\nIf the suspect code passes its unit tests, are the tests complete enough? What happens if you run the tests with this data?\nDo the conditions that caused this bug exist anywhere else in the system? Are there other bugs still in the larval stage, just waiting to hatch?\n\nThe Pragmatic Programmer - 20th Anniversary Edition (page: 229）\n\n #The_Pragmatic_Programmer_-_20th_Anniversary_Edition #koreader"}

The response headers are:

HTTP/1.1 200 OK
Content-Length: 3142
Content-Type: application/json
Date: Thu, 13 Jun 2024 04:10:18 GMT
Grpc-Metadata-Content-Type: application/grpc
Permissions-Policy: geolocation=(self), vibrate=(self), microphone=(), camera=(), fullscreen=*
Referrer-Policy: same-origin
Strict-Transport-Security: max-age=315360000; includeSubDomains; preload
Vary: Accept-Encoding,Origin
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 1; mode=block
Connection: close

Here is the memos server JSON reply:

{"name":"memos/27", "uid":"MXNpoJ8un8qun7EJSKvZW2", "rowStatus":"ACTIVE", "creator":"users/1", "createTime":"2024-06-13T04:10:18Z", "updateTime":"2024-06-13T04:10:18Z", "displayTime":"2024-06-13T04:10:18Z", "content":"Debugging Checklist\nIs the problem being reported a direct result of the underlying bug, or merely a symptom?\nIs the bug really in the framework you’re using? Is it in the OS? Or is it in your code?\nIf you explained this problem in detail to a coworker, what would you say?\nIf the suspect code passes its unit tests, are the tests complete enough? What happens if you run the tests with this data?\nDo the conditions that caused this bug exist anywhere else in the system? Are there other bugs still in the larval stage, just waiting to hatch?\n\nThe Pragmatic Programmer - 20th Anniversary Edition (page: 229）\n\n #The_Pragmatic_Programmer_-_20th_Anniversary_Edition #koreader", "nodes":[{"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":"Debugging Checklist"}}]}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":"Is the problem being reported a direct result of the underlying bug, or merely a symptom?"}}]}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":"Is the bug really in the framework you’re using? Is it in the OS? Or is it in your code?"}}]}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":"If you explained this problem in detail to a coworker, what would you say?"}}]}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":"If the suspect code passes its unit tests, are the tests complete enough? What happens if you run the tests with this data?"}}]}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":"Do the conditions that caused this bug exist anywhere else in the system? Are there other bugs still in the larval stage, just waiting to hatch?"}}]}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":"The Pragmatic Programmer - 20th Anniversary Edition (page: 229）"}}]}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"LINE_BREAK", "lineBreakNode":{}}, {"type":"PARAGRAPH", "paragraphNode":{"children":[{"type":"TEXT", "textNode":{"content":" "}}, {"type":"TAG", "tagNode":{"content":"The_Pragmatic_Programmer_-_20th_Anniversary_Edition"}}, {"type":"TEXT", "textNode":{"content":" "}}, {"type":"TAG", "tagNode":{"content":"koreader"}}]}}], "visibility":"PRIVATE", "tags":[], "pinned":false, "resources":[], "relations":[], "reactions":[], "property":{"tags":["The_Pragmatic_Programmer_-_20th_Anniversary_Edition", "koreader"], "hasLink":false, "hasTaskList":false, "hasCode":false, "hasIncompleteTasks":false}}

[landure]

After investigation, the issue is caused by the table sink. A solution is, on line 41, use table.concat:

    local response = json.decode(table.concat(sink))

I've tested it. It works. It doesn't solve the issue of the unprotected call to json.decode.