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

Allow to set WebPush automatic padding in config #176

Merged
merged 9 commits into from
Feb 27, 2025

Conversation

goaround
Copy link
Contributor

@goaround goaround commented Aug 4, 2022

To support Firefox Android, automatic padding applied by WebPush has to be disabled: https://github.com/web-push-libs/web-push-php#how-can-i-disable-or-customize-automatic-padding

This PR just applies the default 3052 bytes but you can set automatic_padding to false to support Firefox Android

Fix for #103 (comment)

@imrodrigoalves
Copy link

imrodrigoalves commented Nov 25, 2022

Using the approach suggested sets a lower padding value for every notification sent using the current webpush instance. To mitigate this only to reduce the padding on firefox v1 I restricted the lowering of the padding to the subscriptions that have a Firefox v1 endpoint.

Here's what I did:

  1. Extend the default WebPush class
<?php

namespace App\Providers\Models;

use Illuminate\Support\Str;
use Minishlink\WebPush\Encryption;
use Minishlink\WebPush\SubscriptionInterface;
use Minishlink\WebPush\WebPush as MWWebpush;

class WebPush extends MWWebpush
{
    // Solves:
    // https://github.com/laravel-notification-channels/webpush/issues/103
    // https://github.com/web-push-libs/web-push-php/issues/108#issuecomment-545977603
    
    // Had to lower the Mobile Firefox padding for it to work. It probably can go higher but no longer works under 2847 as  suggested.
    const MAX_MOBILE_FIREFOX_PADDING = 2048;
    
    public function queueNotification(SubscriptionInterface $subscription, ?string $payload = null, array $options = [], array $auth = []): void
    {
        $this->setAutomaticPadding(Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH);
        
        if(Str::startsWith($subscription->getEndpoint(), 'https://updates.push.services.mozilla.com/wpush/v1')){
            $this->setAutomaticPadding(self::MAX_MOBILE_FIREFOX_PADDING);
        }
        
        parent::queueNotification($subscription, $payload, $options, $auth);
    }
}
  1. Overwrite the WebPush class loaded by the WebPushServiceProvider
<?php

namespace App\Providers;

use App\Providers\Models\WebPush;
use Minishlink\WebPush\WebPush as MWWebPush;
use NotificationChannels\WebPush\WebPushChannel;
use NotificationChannels\WebPush\WebPushServiceProvider as NCWebPushServiceProvider;

class WebPushServiceProvider extends NCWebPushServiceProvider
{
    /**
     * Register services.
     *
     * @return void
     */
    public function register()
    {
        
    }

    /**
     * Bootstrap services.
     *
     * @return void
     */
    public function boot()
    {
        parent::boot();
        
        $this->app->when(WebPushChannel::class)
        ->needs(MWWebPush::class)
        ->give(function () {
            return (new WebPush(
                $this->webPushAuth(), [], 30, config('webpush.client_options', [])
            ))->setReuseVAPIDHeaders(true);
        });
    }
}
  1. Enable the service provide in config/app.php

  2. Disable automatic loading of the service provider in composer.json

"extra": {
        "laravel": {
            "dont-discover": [
                "laravel-notification-channels/webpush"
            ]
        }
    },

I'll assume some maintenance responsibility over time yet I am completely able to bypass the firefox problem and retain max padding compatibility.

@joostdebruijn
Copy link
Collaborator

I changed this PR a bit to follow the defaults set by web-push-php, which makes this change non-breaking for existing users, and add a env var to configure it.

@joostdebruijn joostdebruijn merged commit 2dc3fb2 into laravel-notification-channels:master Feb 27, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants