Skip to content

Fix issue #9187 Handle wallabag server url with trailing slash(es) #10715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

clach04
Copy link
Contributor

@clach04 clach04 commented Jul 20, 2023

This change is Reviewable

@clach04
Copy link
Contributor Author

clach04 commented Jul 20, 2023

This code is 100% untested with KoReader ;-)

It's been tested in isolation in a lua interpreter on my desktop though.

#!/usr/bin/lua
my_str = "http://somesite.com////";
print(my_str);
print("stripping")
while my_str:sub(-1) == "/" do
  my_str = my_str:sub(1, -2);
end
print(my_str);

@hius07
Copy link
Member

hius07 commented Jul 20, 2023

Maybe simpler my_str = my_str:gsub("/*$", "")

@Frenzie
Copy link
Member

Frenzie commented Jul 20, 2023

I meant a bug with the server configuration really.

Comment on lines 66 to 69
-- Remove any trailing slashes to avoid failures
while self.server_url:sub(-1) == "/" do
self.server_url = self.server_url:sub(1, -2);
end
Copy link
Member

@Frenzie Frenzie Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @hius07 wrote you'd want to collapse and remove any number of / if you did this, but I'd rather do so on entry, so if this causes any issues it's relatively easy to diagnose ("the program keeps removing my slashes!") rather than completely hidden.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So over here:

self.server_url = myfields[1]

@Frenzie Frenzie added this to the 2023.07 milestone Jul 20, 2023
@Frenzie Frenzie added the Plugin label Jul 20, 2023
@clach04
Copy link
Contributor Author

clach04 commented Jul 21, 2023

Thanks @hius07 and @Frenzie for the gentle feedback!

I pushed a replacement change with the two suggestions: cleaner/smaller strip code and in the config area.

From my perspective this is a bug in Wallabag (server), it is really sensitive, but we can clean up the config client side :-)

@clach04
Copy link
Contributor Author

clach04 commented Jul 27, 2023

@Frenzie anything else I need to do on this one?

@Frenzie Frenzie merged commit 500eadf into koreader:master Jul 27, 2023
@Frenzie
Copy link
Member

Frenzie commented Jul 27, 2023

Yup, hope that it doesn't lead to trouble. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants