Skip to content
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

trailing slash is truncated on links depending on the host name #777

Closed
4 tasks done
kaybinwang opened this issue Mar 21, 2023 · 10 comments
Closed
4 tasks done

trailing slash is truncated on links depending on the host name #777

kaybinwang opened this issue Mar 21, 2023 · 10 comments
Labels
🐛 Bug Something isn't working 🙋 Help wanted Extra attention is needed

Comments

@kaybinwang
Copy link

kaybinwang commented Mar 21, 2023

Environment

k8s

Version

0.11.5

Describe the problem

Seems related to #635.

I'm running into an issue where trailing slashes get omitted from the configured URL's. For example, the service URL in the config is http://service.com/health/ but gets rewritten to http://service.com/health when rendering the HTML. This causes issues since the service is only registered behind one of the URLs.

Here's the steps I took to reproduce:

  1. Spin up server with a config that has an app URL of http://10.0.0.200/pihole/admin/
  2. Visit the server using the same hostname @ 10.0.0.200

Expected: app URL in rendered HTML is http://10.0.0.200/pihole/admin/
Actual: app URL in rendered HTML is http://10.0.0.200/pihole/admin (missing backslash)

Now here's where it gets interesting. If I modify /etc/hosts/ to create a new name entry for 10.0.0.200 (e.g. homelab-dev.com) then the error doesn't happen.

For example:

  1. Spin up server with a config that has an app URL of http://10.0.0.200/pihole/admin/
  2. Create a new entry in /etc/hosts for 10.0.0.200 homelab-dev.com
  3. Visit the server using the a different hostname @ homelab-dev.com (still same backend)
  4. Outcome: app URL in rendered HTML is http://10.0.0.200/pihole/admin/ (the correct behavior)

My guess is that the app is using some URL formatting library that is giving inconsistent behavior when the browserUrl.domain == appUrl.domain.

Logs

N/A -- the logs are not helpful b/c the error is most likely caused by something subtle

Context

Scenario A: Talk to the server using a domain that's the same as the app URLs

Screen Shot 2023-03-21 at 2 58 58 AM

Scenario B: Talk to the server using a domain that's different from the app URLs

Screen Shot 2023-03-21 at 2 59 38 AM

Please tick the boxes

  • I confirm that I attached the proper logs
  • I've read the docs
  • I've checked for duplicate issues
  • I've tried to debug myself
@kaybinwang kaybinwang added the 🐛 Bug Something isn't working label Mar 21, 2023
@manuel-rw
Copy link
Collaborator

Thank you for the detailed description!
We'll look into this

@cybercorey
Copy link

I see to have the same issue :)

@manuel-rw manuel-rw added the 🙋 Help wanted Extra attention is needed label May 21, 2023
@manuel-rw manuel-rw pinned this issue May 21, 2023
@manuel-rw
Copy link
Collaborator

Pinning this now for extra attention. We'll investigate why this is happening...

@svart
Copy link

svart commented Aug 31, 2023

I have the exactly same issue on version 0.13.2.

UPD: the only thing worked for me is to use address http://{host_name_here}/pihole/admin/index.php in External address field in tile configuration.

@hskrtich
Copy link
Contributor

I am still running into this issue as well.

@hskrtich
Copy link
Contributor

hskrtich commented Sep 28, 2023

This issue is still happening in v0.13.4 so I have been trying to track down the cause. From what I can tell the change is happening somewhere in the mantine library since the url that is being fed into the UnstyledButton is correct with the trailing slash. I will do some more digging. But I am no react developer so I could be wrong.

@hskrtich
Copy link
Contributor

OK, the issue is coming from using the link component.
https://github.com/ajnart/homarr/blob/dev/src/components/Dashboard/Tiles/Apps/AppTile.tsx#L90

If you set it to use component="a" it fixes the issue but I don't know if that will break other things.

@manuel-rw
Copy link
Collaborator

manuel-rw commented Sep 28, 2023

Hi @hskrtich , thanks for the investigation. As far as I know, next/link would be incorrect here anyway, as it should only be used for internal URLs to avoid reloading the entire page and to use the Next.js router instead.

Do you want to contribute this fix with a pull request? Be advised, that the development branch has some undocumented steps that developers must take. @Tagaishi could you maybe document these in the developer guidelines in the documentation or create a markdown file with that information? If you don't have time, I'll do it myself tonight.

@hskrtich
Copy link
Contributor

Hey @manuel-rw , I can put up a PR if you like but while I am decent at debugging I am no js developer. So if this change just requires the one line change I can do that but if its more complicated then that I will be out of my depth.

@manuel-rw
Copy link
Collaborator

No worries. Thank you for letting is know though. I think it should only be this single line.
component={Link} to component="a". If you don't want to / can't do it, let us know and we'll take care of it. Both I and the other maintainers will soon be on work, so it makes sense if you quickly contribute the fix when it's that simple.

@ajnart ajnart unpinned this issue Oct 9, 2023
ajnart added a commit that referenced this issue Nov 10, 2023
Co-authored-by: Thomas Camlong <thomascamlong@gmail.com>
Co-authored-by: Tagaishi <Tagaishi@hotmail.ch>
Co-authored-by: Manuel <manuel.ruwe@bluewin.ch>
Co-authored-by: Meier Lukas <meierschlumpf@gmail.com>
Co-authored-by: Manuel <30572287+manuel-rw@users.noreply.github.com>
Co-authored-by: Tobias Stadler <28538704+devtobi@users.noreply.github.com>
Co-authored-by: Henry Skrtich <1214484+hskrtich@users.noreply.github.com>
Co-authored-by: AuthorShin <a.saneie@yahoo.com>
Co-authored-by: Diogo Valentim <diogovalentte10@gmail.com>
Co-authored-by: Someone <10882916+InterN0te@users.noreply.github.com>
Co-authored-by: Spillebulle <46653946+Spillebulle@users.noreply.github.com>
Co-authored-by: Justijn Depover <justijndepover@gmail.com>
Fix locale for calendar and clock (#1330)
fix (#1375)
close modal on click when opened (#1396)
fix (#1401)
fix: stop triming traling slashes (#1435)
fixes #777
fix: trim media server url (#1438)
Fix miscellaneous console errors (#1418)
fixes (#1419)
Fix tiptap url CVE (#1459)
Fix allow guest issue (#1472)
Fix the leading slash when adding container via docker (#1478)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working 🙋 Help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants