Skip to content

Commit d9cf35b

Browse files
committed
Address Copilot review feedback
- Fix Link header regex to handle whitespace per RFC 8288 - Add XML security: disable external entities and network access - Add null check for $xml->channel in RSS with Atom namespace - Replace str_starts_with() with strpos() for PHP 7.2 compatibility
1 parent eb3023a commit d9cf35b

2 files changed

Lines changed: 19 additions & 11 deletions

File tree

docs/subscriber-api.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ When a hub sends a verification request, you must confirm the subscription via t
4444
```php
4545
add_filter( 'websub_verify_subscription', function( $allow, $subscription_id, $topic, $mode ) {
4646
// Only verify subscriptions from your plugin
47-
if ( str_starts_with( $subscription_id, 'my-plugin-' ) ) {
47+
if ( 0 === strpos( $subscription_id, 'my-plugin-' ) ) {
4848
// Verify this matches a subscription you requested
4949
$stored_topic = get_option( 'my_plugin_sub_' . $subscription_id );
5050
if ( $stored_topic === $topic ) {
@@ -61,7 +61,7 @@ If you provided a secret when subscribing, you must return it for signature veri
6161

6262
```php
6363
add_filter( 'websub_subscription_secret', function( $secret, $subscription_id ) {
64-
if ( str_starts_with( $subscription_id, 'my-plugin-' ) ) {
64+
if ( 0 === strpos( $subscription_id, 'my-plugin-' ) ) {
6565
return get_option( 'my_plugin_secret_' . $subscription_id );
6666
}
6767
return $secret;
@@ -74,7 +74,7 @@ When the hub delivers new content, handle it via the `websub_received` action:
7474

7575
```php
7676
add_action( 'websub_received', function( $subscription_id, $topic, $content, $content_type ) {
77-
if ( str_starts_with( $subscription_id, 'my-plugin-' ) ) {
77+
if ( 0 === strpos( $subscription_id, 'my-plugin-' ) ) {
7878
// Process the feed content
7979
$feed = simplexml_load_string( $content );
8080
// Update local cache, notify users, etc.
@@ -122,7 +122,7 @@ class My_Feed_Reader {
122122
* Verify subscription requests.
123123
*/
124124
public function verify( $allow, $subscription_id, $topic, $mode ) {
125-
if ( ! str_starts_with( $subscription_id, 'my-reader-' ) ) {
125+
if ( 0 !== strpos( $subscription_id, 'my-reader-' ) ) {
126126
return $allow;
127127
}
128128

@@ -134,7 +134,7 @@ class My_Feed_Reader {
134134
* Provide secret for signature verification.
135135
*/
136136
public function get_secret( $secret, $subscription_id ) {
137-
if ( str_starts_with( $subscription_id, 'my-reader-' ) ) {
137+
if ( 0 === strpos( $subscription_id, 'my-reader-' ) ) {
138138
return get_option( 'my_reader_secret_' . $subscription_id, '' );
139139
}
140140
return $secret;
@@ -144,7 +144,7 @@ class My_Feed_Reader {
144144
* Handle subscription verification success.
145145
*/
146146
public function subscription_verified( $subscription_id, $topic, $lease_seconds, $mode ) {
147-
if ( ! str_starts_with( $subscription_id, 'my-reader-' ) ) {
147+
if ( 0 !== strpos( $subscription_id, 'my-reader-' ) ) {
148148
return;
149149
}
150150

@@ -160,7 +160,7 @@ class My_Feed_Reader {
160160
* Handle received content.
161161
*/
162162
public function handle_content( $subscription_id, $topic, $content, $content_type ) {
163-
if ( ! str_starts_with( $subscription_id, 'my-reader-' ) ) {
163+
if ( 0 !== strpos( $subscription_id, 'my-reader-' ) ) {
164164
return;
165165
}
166166

includes/class-subscriber.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,8 @@ protected static function parse_link_header( $header, $rel ) {
250250
$headers = is_array( $header ) ? $header : array( $header );
251251

252252
foreach ( $headers as $link ) {
253-
// Match pattern: <URL>; rel="value" or <URL>; rel=value.
254-
if ( \preg_match( '/<([^>]+)>;\s*rel=["\']?' . \preg_quote( $rel, '/' ) . '["\']?/i', $link, $matches ) ) {
253+
// Match pattern: <URL>; rel="value" or <URL>; rel=value (with optional whitespace per RFC 8288).
254+
if ( \preg_match( '/<([^>]+)>\s*;\s*rel=["\']?' . \preg_quote( $rel, '/' ) . '["\']?/i', $link, $matches ) ) {
255255
return $matches[1];
256256
}
257257
}
@@ -269,7 +269,15 @@ protected static function parse_link_header( $header, $rel ) {
269269
protected static function parse_feed_for_hub( $content ) {
270270
// Try to parse as XML (Atom/RSS).
271271
\libxml_use_internal_errors( true );
272-
$xml = \simplexml_load_string( $content );
272+
273+
// Disable external entity loading for security (prevents XXE attacks).
274+
// phpcs:ignore PHPCompatibility.FunctionUse.RemovedFunctions.libxml_disable_entity_loaderDeprecated
275+
if ( \PHP_VERSION_ID < 80000 && \function_exists( 'libxml_disable_entity_loader' ) ) {
276+
\libxml_disable_entity_loader( true );
277+
}
278+
279+
// Parse with LIBXML_NONET to prevent network access during parsing.
280+
$xml = \simplexml_load_string( $content, 'SimpleXMLElement', \LIBXML_NONET );
273281

274282
if ( false !== $xml ) {
275283
// Check for Atom namespace.
@@ -286,7 +294,7 @@ protected static function parse_feed_for_hub( $content ) {
286294
}
287295

288296
// RSS with Atom namespace.
289-
if ( isset( $namespaces['atom'] ) ) {
297+
if ( isset( $namespaces['atom'] ) && isset( $xml->channel ) ) {
290298
$atom = $xml->channel->children( $namespaces['atom'] );
291299
foreach ( $atom->link as $link ) {
292300
$attrs = $link->attributes();

0 commit comments

Comments
 (0)