Merge pull request #85 from ben-xo/fix/audit-bugs-2026-04

Fix six audit findings; bring tests up to PHP 8.5

Author: ben-xo

.github/workflows/testing.yml

@@ -4,8 +4,8 @@ jobs:
   build:
     strategy:
       matrix:
-        operating-system: [ubuntu-latest, macos-11]
-        php-versions: ['7.3', '7.4', '8.0', '8.1', '8.2']
+        operating-system: [ubuntu-latest, macos-latest]
+        php-versions: ['7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5']
     runs-on: ${{ matrix.operating-system }}
     steps:
       - name: Setup PHP and extensions

CHANGELOG.txt

@@ -1,6 +1,29 @@
 Changelog
 =========
 
+1.39 2026-04-28 * Fix: ITUNES_BLOCK now only emits <itunes:block> for the
+                  documented values "yes"/"Yes". Previously any non-empty
+                  value (including the example "no" from dir2cast.ini)
+                  would emit the tag, contradicting the docs and the spec.
+                * Fix: the per-MP3-dir dir2cast.ini's mtime now invalidates
+                  the cache. Editing a per-podcast ini file no longer
+                  silently serves stale cached output.
+                * Fix: HTTP Last-Modified header now uses RFC 7231 IMF-fixdate
+                  ("... GMT") instead of RFC 2822 ("... +0000"). The channel's
+                  <lastBuildDate> still uses RFC 2822 as required by RSS.
+                * Fix: HTTP Last-Modified falls back to the cache file's
+                  mtime when the cached feed has no extractable
+                  <lastBuildDate> (avoids emitting an empty header).
+                * Fix: Caching_getID3_Podcast_Helper::addNamespaceTo now
+                  delegates to the wrapped helper's addNamespaceTo (was
+                  incorrectly calling appendToChannel).
+                * Security: ?force=... password compare now uses
+                  hash_equals() with explicit string casts, avoiding
+                  PHP's numeric-string coercion in == and timing leaks.
+                * Tests now run on PHP 8.4 and 8.5 (E_STRICT removal,
+                  getopt() argv-injection change). CI matrix expanded
+                  to cover 8.3, 8.4 and 8.5.
+
 1.38 2023-01-05 * Add <itunes:type> defaulting to "episodic". The ITUNES_TYPE
                   option in dir2cast.ini will let you set the  feed to
                   <itunes:type>serial<itunes:type> set, and then,

README.md

@@ -1,7 +1,7 @@
 [![Testing dir2cast](https://github.com/ben-xo/dir2cast/actions/workflows/testing.yml/badge.svg)](https://github.com/ben-xo/dir2cast/actions/workflows/testing.yml)
 
 
-dir2cast by Ben XO v1.38 (2023-01-05)
+dir2cast by Ben XO v1.39 (2026-04-28)
 ================================================================================
 
 https://github.com/ben-xo/dir2cast/

dir2cast.ini

@@ -107,11 +107,12 @@
 
 ; Whether or not to include the podcast in the iTunes podcast directory.
 ; See https://github.com/simplepie/simplepie-ng/wiki/Spec:-iTunes-Podcast-RSS
-; Valid values are "Yes" or "yes". Any other value will be ignored.
+; Valid values are "Yes" or "yes". Any other value (including the default of
+; unset, or e.g. "no") will be ignored and the tag will be omitted from the feed.
 ;
-; Only set this if you want the feed to be private and completely excluded from 
+; Only set this if you want the feed to be private and completely excluded from
 ; any podcast directory scanning.
-;ITUNES_BLOCK = "no"
+;ITUNES_BLOCK = "yes"
 
 
 ; *** INFORMATION ABOUT YOUR PODCAST - the following can be set using text files ***

dir2cast.php

@@ -56,7 +56,7 @@
 /* DEFAULTS *********************************************/
 
 // error handler needs these, so let's set them now.
-define('VERSION', '1.38');
+define('VERSION', '1.39');
 define('DIR2CAST_HOMEPAGE', 'https://github.com/ben-xo/dir2cast/');
 define('GENERATOR', 'dir2cast ' . VERSION . ' by Ben XO (' . DIR2CAST_HOMEPAGE . ')');
 
@@ -234,7 +234,7 @@ public function appendToChannel(DOMElement $d, DOMDocument $doc) {
     }
 
     public function addNamespaceTo(DOMElement $d, DOMDocument $doc) {
-        return $this->wrapped_helper->appendToChannel($d, $doc);
+        return $this->wrapped_helper->addNamespaceTo($d, $doc);
     }
 
     public function appendToItem(DOMElement $d, DOMDocument $doc, RSS_Item $item)
@@ -390,7 +390,9 @@ public function appendToChannel(DOMElement $channel, DOMDocument $doc)
                 ->appendChild( new DOMText( $this->explicit ) );
         }
 
-        if(!empty($this->block))
+        // Per Apple's spec, <itunes:block> is only meaningful when set to "Yes"/"yes";
+        // any other value must be ignored, so omit the tag entirely.
+        if(!empty($this->block) && strtolower($this->block) === 'yes')
         {
             $channel->appendChild( $doc->createElement('itunes:block') )
                 ->appendChild( new DOMText( $this->block ) );
@@ -1072,11 +1074,32 @@ public function addHelper(Podcast_Helper $helper)
 
     public function http_headers($content_length)
     {
-        // The correct content type is application/rss+xml; however, the de-facto standard is now text/xml. 
+        // The correct content type is application/rss+xml; however, the de-facto standard is now text/xml.
         // See https://stackoverflow.com/questions/595616/what-is-the-correct-mime-type-to-use-for-an-rss-feed
         header('Content-type: text/xml; charset=UTF-8');
         header('Content-length: ' . $content_length);
-        header('Last-modified: ' . $this->getLastBuildDate());
+        $http_last_modified = $this->httpLastModified();
+        if($http_last_modified !== null) {
+            header('Last-modified: ' . $http_last_modified);
+        }
+    }
+
+    /**
+     * Returns the last-modified timestamp formatted as an HTTP-date (IMF-fixdate, RFC 7231)
+     * suitable for the HTTP `Last-Modified` header. The channel's <lastBuildDate> uses
+     * RFC 2822 format (date('r')) which is correct for RSS but not for HTTP headers.
+     */
+    public function httpLastModified()
+    {
+        $lbd = $this->getLastBuildDate();
+        if(!$lbd) {
+            return null;
+        }
+        $ts = strtotime((string)$lbd);
+        if($ts === false) {
+            return null;
+        }
+        return gmdate('D, d M Y H:i:s', $ts) . ' GMT';
     }
 
     /**
@@ -1526,6 +1549,12 @@ public function generate()
             {
                 $this->setLastBuildDate($matches[1]);
             }
+            else
+            {
+                // Fall back to the cache file's mtime so HTTP Last-Modified is still
+                // populated (e.g. if the cached XML predates this format or is corrupt).
+                $this->setLastBuildDate(date('r', filemtime($this->temp_file)));
+            }
         }
         else
         {
@@ -1864,11 +1893,16 @@ public static function bootstrap(array $SERVER, array $GET, array $argv)
     public static function defaults(array $SERVER)
     {
         // if an MP3_DIR specific config file exists, load it now, as long as it's not the same file as the global one!
-        if( 
-            file_exists( MP3_DIR() . 'dir2cast.ini' ) and    
-            realpath(DIR2CAST_BASE() . 'dir2cast.ini') != realpath( MP3_DIR() . 'dir2cast.ini' ) 
+        if(
+            file_exists( MP3_DIR() . 'dir2cast.ini' ) and
+            realpath(DIR2CAST_BASE() . 'dir2cast.ini') != realpath( MP3_DIR() . 'dir2cast.ini' )
         ) {
-            self::load_from_ini( MP3_DIR() . 'dir2cast.ini' );
+            $local_ini = MP3_DIR() . 'dir2cast.ini';
+            self::load_from_ini( $local_ini );
+            // Track this so the dispatcher can use its mtime for cache invalidation;
+            // INI_FILE alone only covers the install-wide ini.
+            if(!defined('LOCAL_INI_FILE'))
+                define('LOCAL_INI_FILE', $local_ini);
         }
 
         self::finalize();
@@ -2100,8 +2134,15 @@ public function __construct(Locking_Cached_Dir_Podcast $podcast)
 
     public function uncache_if_forced($force_password, $get)
     {
-        if( strlen($force_password) && isset($get['force']) && $force_password == $get['force'] )
-        {
+        // Cast both sides to string and use hash_equals to avoid two issues:
+        // (1) PHP's `==` does numeric coercion of numeric-looking strings (so e.g.
+        //     "01" loosely equals "1"); (2) `==`/`===` are not constant-time, so
+        //     they leak password length / prefix via timing.
+        if(
+            strlen((string)$force_password) &&
+            isset($get['force']) &&
+            hash_equals((string)$force_password, (string)$get['force'])
+        ) {
             $this->podcast->uncache();
         }
     }
@@ -2121,6 +2162,8 @@ public function update_mtime_if_dir2cast_or_settings_modified()
         $this->podcast->updateMaxMtime(filemtime(__FILE__), __FILE__);
         if(defined('INI_FILE'))
             $this->podcast->updateMaxMtime(filemtime(INI_FILE), INI_FILE);
+        if(defined('LOCAL_INI_FILE'))
+            $this->podcast->updateMaxMtime(filemtime(LOCAL_INI_FILE), LOCAL_INI_FILE);
     }
 
     public function update_mtime_if_metadata_files_modified()

test/Cached_Dir_PodcastTest.php

@@ -168,6 +168,43 @@ public function test_lastBuildDate_is_valid_whether_served_from_cache_or_not()
         $this->assertEquals($lastBuildDate, $mp3->getLastBuildDate());
     }
 
+    public function test_lastBuildDate_falls_back_to_cache_filemtime_when_unparseable()
+    {
+        // If the cached XML doesn't contain an extractable <lastBuildDate> (corrupted,
+        // truncated, manually edited, or written by an older version that didn't
+        // emit one), the served HTTP `Last-Modified` header would be empty. In that
+        // case we should fall back to the cache file's mtime so the header is still sane.
+        $this->createTestItems();
+        age_dir_by('.', 3600);
+
+        $mp = $this->newPodcast();
+        $mp->generate();
+        unset($mp);
+
+        // Corrupt the cached file so the regex can't extract lastBuildDate.
+        $cache_files = glob(temp_xml_glob());
+        $this->assertNotEmpty($cache_files);
+        $cache_file = $cache_files[0];
+        $corrupted = preg_replace(
+            '#<lastBuildDate>.*?</lastBuildDate>#',
+            '',
+            file_get_contents($cache_file)
+        );
+        file_put_contents($cache_file, $corrupted);
+        $expected_ts = filemtime($cache_file);
+
+        $mp2 = $this->newPodcast(3600);
+        $this->assertTrue($mp2->isCached());
+        $mp2->generate();
+
+        $http_last_modified = $mp2->httpLastModified();
+        $this->assertNotNull($http_last_modified);
+        $this->assertEquals(
+            gmdate('D, d M Y H:i:s', $expected_ts) . ' GMT',
+            $http_last_modified
+        );
+    }
+
     public function tearDown(): void
     {
         file_exists('extra.mp3') && unlink('extra.mp3');

test/Caching_updates_to_local_iniTest.php

@@ -0,0 +1,81 @@
+<?php declare(strict_types=1);
+
+use PHPUnit\Framework\TestCase;
+
+class Caching_updates_to_local_iniTest extends TestCase
+{
+    public $file = 'out.xml';
+    public $output = '';
+    public $returncode = 0;
+
+    public $content = '';
+
+    public function setUp(): void
+    {
+        prepare_testing_dir();
+
+        if(is_link('dir2cast.php')) {
+            $source_file = readlink('dir2cast.php');
+            unlink('dir2cast.php');
+            copy($source_file, 'dir2cast.php');
+        }
+
+        // No global ini for this test - we want to test only the per-MP3-dir ini.
+        if(file_exists('dir2cast.ini')) {
+            unlink('dir2cast.ini');
+        }
+
+        // Pre-populate the media dir with one episode old enough to make it into the
+        // feed. We deliberately do NOT age_dir_by(.) here, because doing so would
+        // change empty.mp3's mtime and thus the item_hash on the next scan, which
+        // would invalidate the cache for reasons unrelated to the local ini.
+        mkdir('media');
+        copy('../fixtures/empty.mp3', 'media/empty.mp3');
+        touch('media/empty.mp3', time() - 86400);
+
+        // First-pass cache must reflect default settings (no local ini yet).
+        exec('php dir2cast.php --media-dir=media --output=out.xml --dont-uncache --min-file-age=30 --ignore-dir2cast-mtime', $this->output, $this->returncode);
+        $this->content = file_get_contents($this->file);
+
+        // Sanity check: the feed used the default language.
+        $this->assertStringContainsString('<language>en-us</language>', $this->content);
+    }
+
+    public function test_per_dir_ini_change_invalidates_cache(): void
+    {
+        // Drop a per-MP3-dir ini that overrides LANGUAGE. Touch it newer than the
+        // cached output but with empty.mp3's mtime untouched (so item_hash stays
+        // the same). The only signal the cache layer has to regenerate is that
+        // the local ini was modified - which is exactly what we want to verify.
+        file_put_contents('media/dir2cast.ini', "LANGUAGE = \"fr-fr\"\n");
+
+        $cache_files = glob('temp' . DIRECTORY_SEPARATOR . '*.xml');
+        $this->assertNotEmpty($cache_files);
+        $cache_file = $cache_files[0];
+
+        // We deliberately do NOT use --ignore-dir2cast-mtime here, so the dispatcher
+        // tracks both dir2cast.php's mtime and the local ini's mtime. To prove the
+        // ini specifically is what triggers invalidation, age dir2cast.php older
+        // than the cache: the only signal newer than the cache is the ini.
+        $now = time();
+        touch('dir2cast.php', $now - 86400);
+        touch($cache_file, $now - 1000);
+        touch('media/dir2cast.ini', $now - 30);
+        clearstatcache();
+
+        exec('php dir2cast.php --media-dir=media --output=out.xml --dont-uncache --min-file-age=30 --debug', $debug_out);
+
+        $new_content = file_get_contents($this->file);
+        $this->assertStringContainsString(
+            '<language>fr-fr</language>',
+            $new_content,
+            "Local dir2cast.ini change must invalidate the cache. Debug output:\n" . implode("\n", $debug_out)
+        );
+    }
+
+    public function tearDown(): void
+    {
+        file_exists($this->file) && unlink($this->file);
+        chdir('..');
+    }
+}