starred/shiori

Fork 0

mirror of https://github.com/go-shiori/shiori.git synced 2026-04-25 14:35:52 +03:00

[GH-ISSUE #459] Crash when bookmark URL is not accessible #285

New issue

Closed

opened 2026-02-25 23:33:52 +03:00 by kerem · 7 comments

kerem commented

2026-02-25 23:33:52 +03:00

Owner

Originally created by @leafee98 on GitHub (Aug 4, 2022).
Original GitHub issue: https://github.com/go-shiori/shiori/issues/459

Step to reproduce:

Start up shiori serve and login
Add a bookmark that is not accessible for shiori
1. If the bookmark is likely i/o timeout, need to click the Update archive on the saved bookmark, the options Keep the old title and excerpt and Update archive as well is not necessary to check or uncheck.
2. If the bookmark is likely connection refused, the shiori will crash immediately.
shiori crash

Log is following:

2022/08/04 08:57:14 error downloading boorkmark: error downloading bookmark: Get "http://127.0.0.1:2323": dial tcp 127.0.0.1:2323: connect: connection refused
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xaaf64a]

goroutine 68 [running]:
github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark.func1()
        /home/runner/work/shiori/shiori/internal/webserver/handler-api.go:329 +0x8a
created by github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark
        /home/runner/work/shiori/shiori/internal/webserver/handler-api.go:324 +0x5c5

The results is same when the error message is i/o timeout, read: connection reset by peer or connection refused like above.

Version

shiori_1.5.3_Linux_x86_64.tar.gz from GitHub release page.

Originally created by @leafee98 on GitHub (Aug 4, 2022). Original GitHub issue: https://github.com/go-shiori/shiori/issues/459 ## Step to reproduce: 1. Start up `shiori serve` and login 2. Add a bookmark that is not accessible for shiori 1. If the bookmark is likely i/o timeout, need to click the `Update archive` on the saved bookmark, the options `Keep the old title and excerpt` and `Update archive as well` is not necessary to check or uncheck. 2. If the bookmark is likely `connection refused`, the shiori will crash immediately. 4. shiori crash ## Log is following: ``` 2022/08/04 08:57:14 error downloading boorkmark: error downloading bookmark: Get "http://127.0.0.1:2323": dial tcp 127.0.0.1:2323: connect: connection refused panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xaaf64a] goroutine 68 [running]: github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark.func1() /home/runner/work/shiori/shiori/internal/webserver/handler-api.go:329 +0x8a created by github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark /home/runner/work/shiori/shiori/internal/webserver/handler-api.go:324 +0x5c5 ``` The results is same when the error message is `i/o timeout`, `read: connection reset by peer` or `connection refused` like above. ## Version `shiori_1.5.3_Linux_x86_64.tar.gz` from GitHub release page.

kerem

2026-02-25 23:33:52 +03:00

closed this issue
added the
type:bug

priority:low

component:backend
labels

kerem commented

2026-02-25 23:33:52 +03:00

Author

Owner

@leafee98 commented on GitHub (Aug 4, 2022):

After crash and restart shiori, there is no offline save or archive for the crashed bookmark. And click Update archive will do nothing, no offline save or archive will be created, and shiori will not crash again for this one bookmark.

@leafee98 commented on GitHub (Aug 4, 2022): After crash and restart shiori, there is no offline save or archive for the crashed bookmark. And click `Update archive` will do nothing, no offline save or archive will be created, and shiori will not crash again for this one bookmark.

kerem commented

2026-02-25 23:33:52 +03:00

Author

Owner

@hulb commented on GitHub (Oct 9, 2022):

@leafee98 Does this issue still exist in the latest release? I try to reproduce it but it seems the issue is gone.

@hulb commented on GitHub (Oct 9, 2022): @leafee98 Does this issue still exist in the latest release? I try to reproduce it but it seems the issue is gone.

kerem commented

2026-02-25 23:33:52 +03:00

Author

Owner

@leafee98 commented on GitHub (Oct 9, 2022):

I think it still exist, I'm on commit d15dc186741c8b8c37949e8b0ef6e7e6273714e8.

I build the shiori by run go get -u -v github.com/go-shiori/shiori and then go build, and get binary shiori.

After configuring database with MariaDB by environment variables like follows.

export SHIORI_DBMS=mysql
export SHIORI_MYSQL_USER=tst
export SHIORI_MYSQL_PASS=test
export SHIORI_MYSQL_NAME=test
export SHIORI_MYSQL_ADDRESS=tcp(127.0.0.1:3306)

Run ./shiori --portable migrate and ./shiori --portable serve.

Then login the web UI and type an inaccessible URL like http://localhost:2323, and I got the final crash log.

INFO[0053] POST /api/bookmarks                           proto=HTTP/1.1 remote="127.0.0.1:50046" reqlen=97 size=223 status=200
2022/10/09 22:18:54 error downloading boorkmark: error downloading bookmark: Get "http://localhost:2323": dial tcp [::1]:2323: connect: connection refused
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xaf85b7]

goroutine 374 [running]:
github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark.func1()
        /home/leafee98/Desktop/project/shiori/internal/webserver/handler-api.go:342 +0x97
created by github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark
        /home/leafee98/Desktop/project/shiori/internal/webserver/handler-api.go:337 +0x62e

If there are any wrong in my building steps or configuration or usage, let me know.

@leafee98 commented on GitHub (Oct 9, 2022): I think it still exist, I'm on commit `d15dc186741c8b8c37949e8b0ef6e7e6273714e8`. I build the shiori by run `go get -u -v github.com/go-shiori/shiori` and then `go build`, and get binary `shiori`. After configuring database with MariaDB by environment variables like follows. ``` export SHIORI_DBMS=mysql export SHIORI_MYSQL_USER=tst export SHIORI_MYSQL_PASS=test export SHIORI_MYSQL_NAME=test export SHIORI_MYSQL_ADDRESS=tcp(127.0.0.1:3306) ``` Run `./shiori --portable migrate` and `./shiori --portable serve`. Then login the web UI and type an inaccessible URL like `http://localhost:2323`, and I got the final crash log. ``` INFO[0053] POST /api/bookmarks proto=HTTP/1.1 remote="127.0.0.1:50046" reqlen=97 size=223 status=200 2022/10/09 22:18:54 error downloading boorkmark: error downloading bookmark: Get "http://localhost:2323": dial tcp [::1]:2323: connect: connection refused panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xaf85b7] goroutine 374 [running]: github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark.func1() /home/leafee98/Desktop/project/shiori/internal/webserver/handler-api.go:342 +0x97 created by github.com/go-shiori/shiori/internal/webserver.(*handler).apiInsertBookmark /home/leafee98/Desktop/project/shiori/internal/webserver/handler-api.go:337 +0x62e ``` If there are any wrong in my building steps or configuration or usage, let me know.

kerem commented

2026-02-25 23:33:52 +03:00

Author

Owner

@hulb commented on GitHub (Oct 9, 2022):

@leafee98 thx, I see. There is an issue there. But it may not causes the main process crash and exit, it should only crash the goroutine.It's nothing to do with your configuration. You could simply add a return after internal/webserver/handler-api.go:340 to fix it. But yes, somewhere nearby the same issue will cause the main process crash.

@hulb commented on GitHub (Oct 9, 2022): @leafee98 thx, I see. There is an issue there. But it may not causes the main process crash and exit, it should only crash the goroutine.It's nothing to do with your configuration. You could simply add a `return` after `internal/webserver/handler-api.go:340` to fix it. But yes, somewhere nearby the same issue will cause the main process crash.

kerem commented

2026-02-25 23:33:52 +03:00

Author

Owner

@fmartingr commented on GitHub (Oct 9, 2022):

There's an insane abuse of panic through all the code, so most of things that can fail just panics. For CLI users it may be ok depending on what it is, but for server users mean a restart of the service, losing the session, etc etc. Error handling and logging requires huge improvements.

@fmartingr commented on GitHub (Oct 9, 2022): There's an insane abuse of `panic` through all the code, so most of things that can fail just panics. For CLI users it may be ok depending on what it is, but for server users mean a restart of the service, losing the session, etc etc. Error handling and logging requires huge improvements.

kerem commented

2026-02-25 23:33:52 +03:00

Author

Owner

@hulb commented on GitHub (Oct 9, 2022):

There's an insane abuse of panic through all the code, so most of things that can fail just panics. For CLI users it may be ok depending on what it is, but for server users mean a restart of the service, losing the session, etc etc. Error handling and logging requires huge improvements.

agree. Panic should be more careful. At present, the web controller layer uses panic to throw out an err and handles it in a recover middlerware. I think it's quite normal, since the err handling in go is annoying.

@hulb commented on GitHub (Oct 9, 2022): > There's an insane abuse of `panic` through all the code, so most of things that can fail just panics. For CLI users it may be ok depending on what it is, but for server users mean a restart of the service, losing the session, etc etc. Error handling and logging requires huge improvements. agree. Panic should be more careful. At present, the web controller layer uses panic to throw out an err and handles it in a recover middlerware. I think it's quite normal, since the err handling in go is annoying.

kerem commented

2026-02-25 23:33:52 +03:00

Author

Owner

@hulb commented on GitHub (Oct 9, 2022):

Maybe a more elegant way is to implement a fail(w *http.Response, err error) function to handle all expected server err and return a more friendly response.

@hulb commented on GitHub (Oct 9, 2022): Maybe a more elegant way is to implement a `fail(w *http.Response, err error)` function to handle all expected server err and return a more friendly response.

kerem referenced this issue

2026-02-25 23:34:43 +03:00

[PR #285] [CLOSED] Add support for search bar to search tags without "tag:" #559