Merge pull request #240 from aik099/download-attachment-test

Fixed the "Api:downloadAttachment" method + added tests

Author: aik099

CHANGELOG.md

@@ -7,14 +7,15 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 ...
 
 ### Changed
-...
+- The `Api::downloadAttachment` method now throws an exception, when attempting to download from a non-Jira website by [@aik099] (#240).
 
 ### Removed
 ...
 
 ### Fixed
 - The `CurlClient` in combination with cURL version < 7.33.0 was getting `426 Upgrade Required` error on any request to the Atlassian Cloud servers by [@aik099] (#239).
 - The `PHPClient` on PHP < 8.0 was getting `426 Upgrade Required` error on any request to the Atlassian Cloud servers by [@aik099] (#239).
+- The `Api::downloadAttachment` method wasn't working with `CurlClient` for Atlassian Cloud servers by [@aik099] (#240).
 
 ## [2.0.0-B1] - 2025-01-04
 ### Added

src/Jira/Api.php

@@ -29,6 +29,7 @@
 use chobie\Jira\Api\Authentication\AuthenticationInterface;
 use chobie\Jira\Api\Client\ClientInterface;
 use chobie\Jira\Api\Client\CurlClient;
+use chobie\Jira\Api\Exception;
 use chobie\Jira\Api\Result;
 
 class Api
@@ -802,23 +803,29 @@ public function api(
 	/**
 	 * Downloads attachment.
 	 *
-	 * @param string $url URL.
+	 * @param string $url URL in "https://your-jira-project.net/rest/api/2/attachment/content/{id}" format.
 	 *
 	 * @return array|string
+	 * @throws Exception When a download URL is coming from a different Jira instance.
+	 * @link   https://developer.atlassian.com/cloud/jira/platform/rest/v2/api-group-issue-attachments/#api-rest-api-2-attachment-content-id-get
 	 */
 	public function downloadAttachment($url)
 	{
-		$result = $this->client->sendRequest(
+		$relative_url = preg_replace('/^' . preg_quote($this->getEndpoint(), '/') . '/', '', $url);
+
+		if ( $relative_url === $url ) {
+			throw new Exception('The download url is coming from the different Jira instance.');
+		}
+
+		return $this->client->sendRequest(
 			self::REQUEST_GET,
-			$url,
+			$relative_url,
 			array(),
-			null,
+			$this->getEndpoint(),
 			$this->authentication,
 			true,
-			false
+			false // Needed for test suite to work (probably Prophecy glitch).
 		);
-
-		return $result;
 	}
 
 	/**

src/Jira/Api/Client/CurlClient.php

@@ -97,6 +97,7 @@ public function sendRequest(
 		curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, false);
 		curl_setopt($curl, CURLOPT_SSL_VERIFYHOST, 0);
 		curl_setopt($curl, CURLOPT_VERBOSE, $debug);
+		curl_setopt($curl, CURLOPT_FOLLOWLOCATION, 1);
 
 		if ( $is_file ) {
 			if ( defined('CURLOPT_SAFE_UPLOAD') && PHP_VERSION_ID < 70000 ) {

src/Jira/Api/Client/PHPClient.php

@@ -122,8 +122,9 @@ public function sendRequest(
 			'http' => array(
 				'method' => $method,
 				'header' => implode("\r\n", $header),
-				'ignore_errors' => true,
+				'ignore_errors' => true, // Not needed since PHP 5.1.0.
 				'protocol_version' => 1.1, // Not needed since PHP 8.0.
+				'follow_location' => 1, // Not needed since PHP 5.1.0.
 			),
 		);
 

tests/Jira/Api/Client/AbstractClientTestCase.php

@@ -144,6 +144,39 @@ public static function fileUploadDataProvider()
 		);
 	}
 
+	public function testFileDownloadWithFollowLocation()
+	{
+		$result = $this->client->sendRequest(
+			Api::REQUEST_GET,
+			'/tests/download_attachment.php?download_url=' . rtrim($_SERVER['REPO_URL'], '/') . '/LICENSE.md',
+			array(),
+			rtrim($_SERVER['REPO_URL'], '/'),
+			new Basic('user1', 'pass1'),
+			true
+		);
+
+		$this->assertStringEqualsFile(
+			$this->getProjectRoot() . '/LICENSE.md',
+			$result
+		);
+	}
+
+	/**
+	 * Returns project root.
+	 *
+	 * @return string
+	 */
+	protected function getProjectRoot()
+	{
+		$dir = __DIR__;
+
+		while ( !file_exists($dir . '/LICENSE.md') ) {
+			$dir = dirname($dir);
+		}
+
+		return $dir;
+	}
+
 	public function testUnsupportedCredentialGiven()
 	{
 		$client_class_parts = explode('\\', get_class($this->client));

tests/Jira/ApiTest.php

@@ -5,6 +5,7 @@
 
 use chobie\Jira\Api;
 use chobie\Jira\Api\Authentication\AuthenticationInterface;
+use chobie\Jira\Api\Exception;
 use chobie\Jira\Api\Result;
 use chobie\Jira\IssueType;
 use Prophecy\Prophecy\ObjectProphecy;
@@ -152,6 +153,33 @@ public function testReleaseVersionParameterMerging()
 		$this->assertFalse($this->api->releaseVersion(111000, $release_date, array('test' => 'extra')));
 	}
 
+	public function testDownloadAttachmentSuccessful()
+	{
+		$expected = 'file content';
+
+		$this->expectClientCall(
+			Api::REQUEST_GET,
+			'/rest/api/2/attachment/content/12345',
+			array(),
+			$expected,
+			true
+		);
+
+		$actual = $this->api->downloadAttachment(self::ENDPOINT . '/rest/api/2/attachment/content/12345');
+
+		if ( $actual !== null ) {
+			$this->assertEquals($expected, $actual);
+		}
+	}
+
+	public function testDownloadAttachmentWithException()
+	{
+		$this->expectException(Exception::class);
+		$this->expectExceptionMessage('The download url is coming from the different Jira instance.');
+
+		$this->api->downloadAttachment('https://other.jira-instance.com/rest/api/2/attachment/content/12345');
+	}
+
 	/**
 	 * @dataProvider createRemoteLinkDataProvider
 	 */

tests/download_attachment.php

@@ -0,0 +1,3 @@
+<?php
+
+header('Location: ' . $_GET['download_url'], true, 303);