go-shiori / shiori

Simple bookmark manager built with Go
MIT License
9.28k stars 551 forks source link

Some API calls do not respect root path #870

Closed ChaosExAnima closed 6 months ago

ChaosExAnima commented 6 months ago

Data

Describe the bug / actual behavior

Some API calls in the settings section of the front end don't use the base path. This includes:

Expected behavior

I expect my settings to save!

To Reproduce

Steps to reproduce the behavior:

  1. Set up an installation on a root path, ex: /shiori.
  2. Log in an admin account and navigate to settings.
  3. Click on 'Show bookmark's ID'
  4. See error

Screenshots

The error message is the HTML of another service I run, so I'd prefer to keep that private.

Notes

I dug around in the code a bit, and the linked lines I believe are the culprit. It looks like there's some inconsistency with API fetches, specifically the initial / for settings and lack of URL entirely for deleting accounts.

I can whip up a PR if needed.

fmartingr commented 6 months ago

Hey @ChaosExAnima, apologies for this. Let's figure out what's going on.

Note: Cann this be another side effect of #865?

ChaosExAnima commented 6 months ago

Hi @fmartingr, yes and yes respectively. To be clear, if those aren't set the app doesn't load correctly- my app is loading and saving bookmarks no problem, but only certain API calls fail as per the title. If you double-check my description, I already linked to what I believe is the problem.

Also as a side note, it's incredibly confusing to have both the SHIORI_HTTP_ROOT_PATH env var and a --webroot argument.

fmartingr commented 6 months ago

Hi @fmartingr, yes and yes respectively. To be clear, if those aren't set the app doesn't load correctly- my app is loading and saving bookmarks no problem, but only certain API calls fail as per the title. If you double-check my description, I already linked to what I believe is the problem.

I just wanted to confirm the problem which seems to be the slash at the beginning of those calls ignoring the root path in the base element.

Also as a side note, it's incredibly confusing to have both the SHIORI_HTTP_ROOT_PATH env var and a --webroot argument.

Yeah, this is a bug, only one should be required, potentially to be fixed in 1.6.1 this week once I merge #865

fmartingr commented 6 months ago

Can you check if this was solved with v1.6.1?

ChaosExAnima commented 6 months ago

@fmartingr - not fixed, no.

fmartingr commented 6 months ago

@fmartingr - not fixed, no.

Ups, sorry, I thought this was another ticket! Let me do a quick PR for this.