Skip to content

Commit

Permalink
Small fixes and cleanup
Browse files Browse the repository at this point in the history
A pair of missing null-checks was detected by Scrutinizer. The rest of the
changes are just stylistic.
  • Loading branch information
paulijar committed Dec 28, 2024
1 parent 8cd5d82 commit 1781a65
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 17 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,7 @@ A mistake made when creating the release package 1.3.0 broke the application pre
- reset-database command
- set length of a track in the database and expose via Ampache
- fix album count in Ampache API
- expose Album cover via (inofficial) Ampache API
- expose Album cover via (unofficial) Ampache API
- ownCloud 8 compatibility
- user interaction needed to start background scan and reload the music view

Expand Down
2 changes: 1 addition & 1 deletion js/shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ OCA.Music.Utils = class {
}

/**
* Format baud rate given as bit per second to kilobits per second with integer precission
* Format baud rate given as bit per second to kilobits per second with integer precision
*/
static formatBitrate(bitsPerSecond : number) : string {
return (bitsPerSecond / 1000).toFixed() + ' kbps';
Expand Down
12 changes: 9 additions & 3 deletions lib/Controller/AmpacheController.php
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,9 @@ protected function system_preference(string $filter) : array {
* @AmpacheAPI
*/
protected function download(int $id, string $type='song', bool $recordPlay=false) : Response {
// TODO: This may return both Ampache error responses and HTTP error responses. Should be refactored to always return
// HTTP error codes on all error cases. This is probably used in a context where Ampache responses are not properly parsed.

// request param `format` is ignored
// request param `recordPlay` is not a specified part of the API
$userId = $this->userId();
Expand All @@ -1550,10 +1553,13 @@ protected function download(int $id, string $type='song', bool $recordPlay=false
}
} elseif ($type === 'podcast' || $type === 'podcast_episode') { // there's a difference between APIv4 and APIv5
$episode = $this->podcastEpisodeBusinessLayer->find($id, $userId);
if ($this->isInternalSession() && $this->config->getSystemValue('music.relay_podcast_stream', true)) {
return new RelayStreamResponse($episode->getStreamUrl());
$streamUrl = $episode->getStreamUrl();
if ($streamUrl === null) {
return new ErrorResponse(Http::STATUS_NOT_FOUND, "The podcast episode $id has no stream URL");
} elseif ($this->isInternalSession() && $this->config->getSystemValue('music.relay_podcast_stream', true)) {
return new RelayStreamResponse($streamUrl);
} else {
return new RedirectResponse($episode->getStreamUrl());
return new RedirectResponse($streamUrl);
}
} elseif ($type === 'playlist') {
$songIds = ($id === self::ALL_TRACKS_PLAYLIST_ID)
Expand Down
5 changes: 4 additions & 1 deletion lib/Controller/PodcastApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,10 @@ public function episodeStream(int $id) {
$episode = $this->podcastService->getEpisode($id, $this->userId);

if ($episode !== null) {
if ($this->config->getSystemValue('music.relay_podcast_stream', true)) {
$streamUrl = $episode->getStreamUrl();
if ($streamUrl === null) {
return new ErrorResponse(Http::STATUS_NOT_FOUND, "The podcast episode $id has no stream URL");
} elseif ($this->config->getSystemValue('music.relay_podcast_stream', true)) {
return new RelayStreamResponse($episode->getStreamUrl());
} else {
return new RedirectResponse($episode->getStreamUrl());
Expand Down
2 changes: 1 addition & 1 deletion lib/Http/RelayStreamResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function callback(IOutput $output) {
$inStream = \fopen($this->url, 'rb', false, $this->context);
$outStream = \fopen('php://output', 'wb');

$bytesCopied = \stream_copy_to_stream($inStream, $outStream);
\stream_copy_to_stream($inStream, $outStream);
\fclose($outStream);
\fclose($inStream);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Utility/HttpUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public static function getUrlHeaders(string $url, $context) : ?array {
if (self::isUrlSchemeOneOf($url, self::ALLOWED_SCHEMES)) {
// the type of the second parameter of get_header has changed in PHP 8.0
$associative = \version_compare(\phpversion(), '8.0', '<') ? 1 : true;
$result = @\get_headers($url, $associative, $context);
$result = @\get_headers($url, /** @scrutinizer ignore-type */ $associative, $context);

if ($result === false) {
$result = null;
Expand Down
18 changes: 9 additions & 9 deletions lib/Utility/StreamTokenService.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
*/
class StreamTokenService {

private Cache $cache;
private Cache $cache;
private ?string $secret;

public function __construct(Cache $cache) {
$this->cache = $cache;
public function __construct(Cache $cache) {
$this->cache = $cache;
$this->secret = null; // lazy load
}
}

public function tokenForUrl(string $url) : string {
$secret = $this->getPrivateSecret();
Expand All @@ -40,7 +40,7 @@ public function urlTokenIsValid(string $url, string $token) : bool {
return self::tokenIsValid($token, $url, $secret);
}

private static function createToken(string $message, string $privateSecret) : string {
private static function createToken(string $message, string $privateSecret) : string {
$salt = \random_bytes(32);
$hash = \hash('sha256', $salt . $message . $privateSecret, /*binary=*/true);
return \base64_encode($salt . $hash);
Expand All @@ -56,10 +56,10 @@ private static function tokenIsValid(string $token, string $message, string $pri
private function getPrivateSecret() : string {
// Load the secret all the way from the DB only once per request and cache it for later
// invocations to avoid flooding DB requests when parsing a large playlist file for Files.
if ($this->secret === null) {
$this->secret = $this->getPrivateSecretFromDb();
}
return $this->secret;
// Make this so that also Scrutinizer understands that the return value is always non-null.
$secret = $this->secret ?? $this->getPrivateSecretFromDb();
$this->secret = $secret;
return $secret;
}

private function getPrivateSecretFromDb() : string {
Expand Down

0 comments on commit 1781a65

Please sign in to comment.