ash-jc-allen / favicon-fetcher

A Laravel package for fetching favicons from websites.
MIT License
167 stars 13 forks source link

Doesn't handle urls that redirects to path #72

Closed Josh-TX closed 8 months ago

Josh-TX commented 8 months ago

When the Uri provided to the Scan function responds with a 301 or 302 redirect, It's not working when the redirect location is a path.

Specifically, I'm having an issue where my local Jellyfin instance is at 192.168.x.x:x, but that address returns a 302 with a location of /web/index.html. The favicons are then publicly available at 192.168.x.x:x/web/asset.ico. However, the ScanResults will have a location of 192.168.x.x:x/asset.ico (missing the web path). The fetch is also returning null, which I believe has the same root issue.

Redirects seem to be handled correctly when it's a different domain (for example, https://youtu.be has some scan results for youtube.com).

Josh-TX commented 8 months ago

As a temporary fix, you can convert the URI to the redirected URI before calling Scan/Fetch using something like this:

 Request.Url = (await httpClient.GetAsync(Request.Url)).RequestMessage.RequestUri;

This circumvents the problem, but it's not ideal because it causes an additional http request

ash-jc-allen commented 8 months ago

Hey @Josh-TX! I'm sorry to hear that you're having problems with the package. Is it possible for you to give me some steps to reproduce this issue (maybe a fresh Laravel repo)?

I'm a little confused because of the mention of the Scan function and the code snippet in your comment. I'm not 100% sure, but it looks very much like C#, but this package is a PHP package 🙂

Josh-TX commented 8 months ago

Oh, my bad... I must've commented on the wrong repo. Yeah, this is an issue with the dotnet nuget package

ash-jc-allen commented 8 months ago

That's no problem haha! 😄