Skip to content

URL-encode branch refs in DevPullCommand GitHub API requests #20

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sak0a merged 2 commits into from
Feb 7, 2026
Merged

URL-encode branch refs in DevPullCommand GitHub API requests #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
561ddc9
Initial plan
Copilot Feb 7, 2026
1aa052f
URL-encode branch names in DevPullCommand to handle slashes
Copilot Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Console/Commands/DevPullCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ public function handle(): int

private function fetchCommitInfo(Client $client, string $repo, string $ref): array
{
$url = self::GITHUB_API_BASE . "/repos/{$repo}/commits/{$ref}";
$encodedRef = rawurlencode($ref);
$url = self::GITHUB_API_BASE . "/repos/{$repo}/commits/{$encodedRef}";

try {
$response = $client->get($url);
Expand Down
195 changes: 195 additions & 0 deletions tests/Unit/Console/DevPullCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
<?php

declare(strict_types=1);

namespace Notur\Tests\Unit\Console;

use GuzzleHttp\Client;
use GuzzleHttp\Handler\MockHandler;
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Middleware;
use GuzzleHttp\Psr7\Response;
use Notur\Console\Commands\DevPullCommand;
use Notur\NoturServiceProvider;
use Orchestra\Testbench\TestCase;

class DevPullCommandTest extends TestCase
{
protected function getPackageProviders($app): array
{
return [NoturServiceProvider::class];
}

protected function getEnvironmentSetUp($app): void
{
$app['config']->set('database.default', 'testing');
$app['config']->set('database.connections.testing', [
'driver' => 'sqlite',
'database' => ':memory:',
'prefix' => '',
]);
$app['config']->set('notur.repository', 'sak0a/notur');
}
Comment on lines +14 to +32

Copilot AI Feb 7, 2026

Copy link

Choose a reason for hiding this comment

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

This test class extends Orchestra\Testbench\TestCase and boots NoturServiceProvider + DB config, but the tests only invoke a private method with a provided Guzzle client and don鈥檛 use the Laravel application/container. Consider switching to PHPUnit\Framework\TestCase (as most unit tests do) and removing getPackageProviders()/getEnvironmentSetUp() to reduce test runtime and avoid unnecessary framework coupling.

Copilot uses AI. Check for mistakes.

public function test_url_encodes_branch_names_with_slashes(): void
{
// Create a mock handler to capture HTTP requests
$container = [];
$history = Middleware::history($container);

$mock = new MockHandler([
new Response(200, [], json_encode([
'sha' => 'abc123def456',
'commit' => [
'message' => 'Test commit',
'author' => [
'name' => 'Test Author',
'date' => '2026-02-07T12:00:00Z',
],
],
])),
]);

$handlerStack = HandlerStack::create($mock);
$handlerStack->push($history);

// Test with a branch name containing slashes
$client = new Client(['handler' => $handlerStack]);

$command = new DevPullCommand();
$reflection = new \ReflectionClass($command);
$method = $reflection->getMethod('fetchCommitInfo');
$method->setAccessible(true);

$method->invoke($command, $client, 'sak0a/notur', 'feature/my-branch');
Comment on lines +36 to +64

Copilot AI Feb 7, 2026

Copy link

Choose a reason for hiding this comment

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

The mock handler + history + reflection setup is duplicated in each test case. Refactoring this into a small helper (e.g., a method that returns [$client, &$container, $invokeFetchCommitInfo]) would reduce repetition and make it easier to add more encoding cases later.

Copilot uses AI. Check for mistakes.

// Verify the URL was properly encoded
$this->assertCount(1, $container);
$request = $container[0]['request'];
$uri = (string) $request->getUri();

// The branch name should be URL-encoded: feature/my-branch -> feature%2Fmy-branch
$this->assertStringContainsString('/commits/feature%2Fmy-branch', $uri);
$this->assertStringNotContainsString('/commits/feature/my-branch', $uri);
}

public function test_url_encodes_branch_names_with_special_characters(): void
{
// Create a mock handler
$container = [];
$history = Middleware::history($container);

$mock = new MockHandler([
new Response(200, [], json_encode([
'sha' => 'xyz789abc',
'commit' => [
'message' => 'Another test',
'author' => [
'name' => 'Test Author',
'date' => '2026-02-07T12:00:00Z',
],
],
])),
]);

$handlerStack = HandlerStack::create($mock);
$handlerStack->push($history);

$client = new Client(['handler' => $handlerStack]);

$command = new DevPullCommand();
$reflection = new \ReflectionClass($command);
$method = $reflection->getMethod('fetchCommitInfo');
$method->setAccessible(true);

// Test with a branch name containing spaces and special characters
$method->invoke($command, $client, 'sak0a/notur', 'feature/my branch-v2');

$this->assertCount(1, $container);
$request = $container[0]['request'];
$uri = (string) $request->getUri();

// The branch name should be URL-encoded
$this->assertStringContainsString('/commits/feature%2Fmy%20branch-v2', $uri);
}

public function test_handles_simple_branch_names_correctly(): void
{
// Create a mock handler
$container = [];
$history = Middleware::history($container);

$mock = new MockHandler([
new Response(200, [], json_encode([
'sha' => '123abc456',
'commit' => [
'message' => 'Simple test',
'author' => [
'name' => 'Test Author',
'date' => '2026-02-07T12:00:00Z',
],
],
])),
]);

$handlerStack = HandlerStack::create($mock);
$handlerStack->push($history);

$client = new Client(['handler' => $handlerStack]);

$command = new DevPullCommand();
$reflection = new \ReflectionClass($command);
$method = $reflection->getMethod('fetchCommitInfo');
$method->setAccessible(true);

// Test with a simple branch name without special characters
$method->invoke($command, $client, 'sak0a/notur', 'main');

$this->assertCount(1, $container);
$request = $container[0]['request'];
$uri = (string) $request->getUri();

// Simple branch names should still work
$this->assertStringContainsString('/commits/main', $uri);
}

public function test_handles_commit_sha(): void
{
// Create a mock handler
$container = [];
$history = Middleware::history($container);

$mock = new MockHandler([
new Response(200, [], json_encode([
'sha' => 'abc123def456789',
'commit' => [
'message' => 'SHA test',
'author' => [
'name' => 'Test Author',
'date' => '2026-02-07T12:00:00Z',
],
],
])),
]);

$handlerStack = HandlerStack::create($mock);
$handlerStack->push($history);

$client = new Client(['handler' => $handlerStack]);

$command = new DevPullCommand();
$reflection = new \ReflectionClass($command);
$method = $reflection->getMethod('fetchCommitInfo');
$method->setAccessible(true);

// Test with a commit SHA (no special characters)
$method->invoke($command, $client, 'sak0a/notur', 'abc123def456789');

$this->assertCount(1, $container);
$request = $container[0]['request'];
$uri = (string) $request->getUri();

// Commit SHAs should work as expected
$this->assertStringContainsString('/commits/abc123def456789', $uri);
}
}