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

Support for Custom and default Favicons in Pode Endpoints #1509

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

mdaneri
Copy link
Contributor

@mdaneri mdaneri commented Mar 1, 2025

Overview

This PR introduces two new parameters to Add-PodeEndpoint, allowing greater control over favicon handling in HTTP/HTTPS endpoints.

Changes

  • -Favicon (byte[]): Allows specifying a custom favicon for an endpoint.
  • -DefaultFavicon (switch): Enable the default Pode favicon for HTTP/HTTPS endpoints.

Usage Examples

# Example: Setting a custom favicon
$iconBytes = [System.IO.File]::ReadAllBytes("C:\path\to\custom.ico")
Add-PodeEndpoint -Address localhost -Port 8080 -Protocol Http -Favicon $iconBytes

# Example: Using the default favicon.ico
Add-PodeEndpoint -Address localhost -Port 8081 -Protocol Http -DefaultFavicon

For more details on favicon formats and specifications, refer to the Favicon specification and RFC 5988.

mdaneri added a commit to mdaneri/Pode that referenced this pull request Mar 2, 2025
mdaneri added a commit to mdaneri/Pode that referenced this pull request Mar 2, 2025
This was referenced Mar 2, 2025
mdaneri added a commit to mdaneri/Pode that referenced this pull request Mar 3, 2025
mdaneri added a commit to mdaneri/Pode that referenced this pull request Mar 3, 2025
mdaneri added a commit to mdaneri/Pode that referenced this pull request Mar 3, 2025
@mdaneri mdaneri mentioned this pull request Mar 3, 2025

# If no -Favicon is provided, the protocol is either HTTP or HTTPS, and -DefaultFavicon is enabled,
# set the default favicon from the Pode module's miscellaneous path.
if ( ($null -eq $Favicon) -and (@('Http', 'Https') -icontains $Protocol) -and $DefaultFavicon) {
Copy link
Owner

Choose a reason for hiding this comment

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

We wouldn't need to check ($null -eq $Favicon) at this point, as we know it will be null/empty if $DefaultFavicon is true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? I can call add-podeEndpoint with both -Favicon adn -DefaultFavicon.
I can create an new parameterset but start to become complicated with all these parameters

@Badgerati Badgerati added enhancement ⬆️ story-points: 3 Low complexity. Small features or minor refactors. Some review required, but fairly straightforward labels Mar 16, 2025
@Badgerati Badgerati added this to the 2.12.1 milestone Mar 16, 2025
mdaneri added 2 commits March 17, 2025 09:47
- ContentType is matching the image type
- Favicon parameter can be a path or a byte array
- Documentation improvements
@Badgerati
Copy link
Owner

I was having a think about this yesterday, and I'm wondering if it's actually better to separate the Favicon logic away from Endpoint. Separating it out into it's own Add-PodeFavicon will allow use to support both differing favicons for Endpoints, but also for Routes as well - as we could have an -EndpointName and -Route parameters, where -Route is either a literal path or path pattern much like on Add-PodeMiddleware.

This helps keep the Endpoint functions tidier, but also lets us more easily extend Favicon support in the future if we need to.

As for where the favicon selector logic runs, currently is directly in the PodeServer.ps1 scriptblock. I'm thinking this would be better either in the Get-PodePublicMiddleware (at the top, before we check /public) - this way all supported web servers get it. Or, we add a new Get-PodeFaviconMiddleware which get run before Get-PodePublicMiddleware.

This would also mean you could have different -Path, -Binary, -Default parameters split by a ParameterSet - so it's more obvious/natural as to what the parameter does.

Ie:

# load file and read in bytes
Add-PodeFavicon
    -Path [string]
    -EndpointName [string]
    -Route [string]

# has bytes directly supplied for custom cases
Add-PodeFavicon
    -Binary [byte[]]
    -EndpointName [string]
    -Route [string]

# use the internal default favicon
Add-PodeFavicon
    -Default [switch]
    -EndpointName [string]
    -Route [string]

EndpointName/Route could be [string[]] if we want. They are also optional, so if Add-PodeFavicon -Default is only supplied, this applies to every request.

Additional functions:

  • Remove-PodeFavicon
  • Test-PodeFavicon
  • Get-PodeFavicon

In terms of the fitlering/priority for selecting the favicon, if the request is /favicon.ico:

  1. If no custom favicons, skip and let /public try and use favicon.ico
  2. If custom favicons:
    a. Filter by EndpointName
    b. Filter by Route on the above EndpointName filtering - if the above returned an empty array then filter on the original array for favicons with no EndpointName
    c. If still no favicons, then get all favicons from the original array with no EndpointName or Route (ie, a favicon for every request)
    c. If favicon found, return custom/default favicon
    e. In the case where multiple favicons match, either choose the last one (ie, the last favicon created), or possibly a -Priority flag on Add-PodeFavicon 🤔

@mdaneri
Copy link
Contributor Author

mdaneri commented Mar 18, 2025

Personally, I believe using middleware to serve the favicon is overkill. The favicon should be served as early as possible—ideally, directly from the C# code—before any route or path processing occurs. This approach aligns with the RFC guidelines, which imply that the favicon should be returned regardless of any security measures or routing rules applied to other endpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ⬆️ story-points: 3 Low complexity. Small features or minor refactors. Some review required, but fairly straightforward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants