Skip to content

Commit 1c39a2e

Browse files
clueclue
committed
Use URI templates to build properly escaped URIs
1 parent a879b63 commit 1c39a2e

File tree

2 files changed

+77
-45
lines changed

2 files changed

+77
-45
lines changed

‎src/Client.php

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,20 @@ public function fetchFile($path, $revision = null)
4141
return Promise\reject(new InvalidArgumentException('File path MUST NOT end with trailing slash'));
4242
}
4343

44-
$url = $path . '?view=co';
45-
if ($revision !== null) {
46-
$url .= '&pathrev=' . $revision;
47-
}
48-
4944
// TODO: fetching a directory redirects to path with trailing slash
5045
// TODO: status returns 200 OK, but displays an error message anyways..
5146
// TODO: see not-a-file.html
5247
// TODO: reject all paths with trailing slashes
5348

54-
return $this->fetch($url);
49+
return $this->fetch(
50+
$this->browser->resolve(
51+
'/{+path}?view=co{&pathrev}',
52+
array(
53+
'path' => ltrim($path, '/'),
54+
'pathrev' => $revision
55+
)
56+
)
57+
);
5558
}
5659

5760
public function fetchDirectory($path, $revision = null, $showAttic = false)
@@ -60,21 +63,19 @@ public function fetchDirectory($path, $revision = null, $showAttic = false)
6063
return Promise\reject(new InvalidArgumentException('Directory path MUST end with trailing slash'));
6164
}
6265

63-
$url = $path;
64-
65-
if ($revision !== null) {
66-
$url .= '?pathrev=' . $revision;
67-
}
68-
69-
if ($showAttic) {
70-
$url .= (strpos($url, '?') === false) ? '?' : '&';
71-
$url .= 'hideattic=0';
72-
}
73-
7466
// TODO: path MUST end with trailing slash
7567
// TODO: accessing files will redirect to file with relative location URL (not supported by clue/buzz-react)
7668

77-
return $this->fetchXml($url)->then(function (SimpleXMLElement $xml) {
69+
return $this->fetchXml(
70+
$this->browser->resolve(
71+
'/{+path}{?pathrev,hideattic}',
72+
array(
73+
'path' => ltrim($path, '/'),
74+
'pathrev' => $revision,
75+
'hideattic' => $showAttic ? '0' : null
76+
)
77+
)
78+
)->then(function (SimpleXMLElement $xml) {
7879
// TODO: reject if this is a file, instead of directory => contains "Log of" instead of "Index of"
7980
// TODO: see is-a-file.html
8081

@@ -84,23 +85,31 @@ public function fetchDirectory($path, $revision = null, $showAttic = false)
8485

8586
public function fetchPatch($path, $r1, $r2)
8687
{
87-
$url = $path . '?view=patch&r1=' . $r1 . '&r2=' . $r2;
88-
89-
return $this->fetch($url);
88+
return $this->fetch(
89+
$this->browser->resolve(
90+
'/{+path}?view=patch{&r1,r2}',
91+
array(
92+
'path' => ltrim($path, '/'),
93+
'r1' => $r1,
94+
'r2' => $r2
95+
)
96+
)
97+
);
9098
}
9199

92100
public function fetchLog($path, $revision = null)
93101
{
94-
$url = $path . '?view=log';
95-
96102
// TODO: invalid revision shows error page, but HTTP 200 OK
97103

98-
if ($revision !== null) {
99-
$url .= (strpos($url, '?') === false) ? '?' : '&';
100-
$url .= 'pathrev=' . $revision;
101-
}
102-
103-
return $this->fetchXml($url)->then(array($this->parser, 'parseLogEntries'));
104+
return $this->fetchXml(
105+
$this->browser->resolve(
106+
'/{+path}?view=log{&pathrev}',
107+
array(
108+
'path' => ltrim($path, '/'),
109+
'pathrev' => $revision
110+
)
111+
)
112+
)->then(array($this->parser, 'parseLogEntries'));
104113
}
105114

106115
public function fetchRevisionPrevious($path, $revision)
@@ -121,9 +130,14 @@ public function fetchAllPreviousRevisions($path)
121130

122131
private function fetchLogXml($path)
123132
{
124-
$url = $path . '?view=log';
125-
126-
return $this->fetchXml($url);
133+
return $this->fetchXml(
134+
$this->browser->resolve(
135+
'/{+path}?view=log',
136+
array(
137+
'path' => ltrim($path, '/')
138+
)
139+
)
140+
);
127141
}
128142

129143
private function fetchXml($url)
@@ -133,7 +147,7 @@ private function fetchXml($url)
133147

134148
private function fetch($url)
135149
{
136-
return $this->browser->get(ltrim($url, '/'))->then(
150+
return $this->browser->get($url)->then(
137151
function (Response $response) {
138152
return (string)$response->getBody();
139153
},

‎tests/ClientTest.php

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,22 @@
44
use Clue\React\Buzz\Message\Response;
55
use Clue\React\Buzz\Message\Body;
66
use React\Promise;
7+
use Clue\React\Buzz\Browser;
8+
use Clue\React\Buzz\Message\Request;
79

810
class ClientTest extends TestCase
911
{
10-
private $browser;
12+
private $uri = 'http://viewvc.example.org/';
13+
private $sender;
1114
private $client;
1215

1316
public function setUp()
1417
{
15-
// test case does not take base URI into account
16-
$this->browser = $this->getMockBuilder('Clue\React\Buzz\Browser')->disableOriginalConstructor()->setMethods(array('get'))->getMock();
17-
$this->client = new Client($this->browser);
18+
$this->sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock();
19+
20+
$browser = new Browser($this->getMock('React\EventLoop\LoopInterface'), $this->sender);
21+
22+
$this->client = new Client($browser->withBase($this->uri));
1823
}
1924

2025
public function testInvalidDirectory()
@@ -32,7 +37,8 @@ public function testInvalidFile()
3237
public function testFetchFile()
3338
{
3439
$response = new Response('HTTP/1.0', 200, 'OK', array(), new Body('# hello'));
35-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co'))->will($this->returnValue(Promise\resolve($response)));
40+
41+
$this->expectRequest($this->uri . 'README.md?view=co')->will($this->returnValue(Promise\resolve($response)));
3642

3743
$promise = $this->client->fetchFile('README.md');
3844

@@ -41,7 +47,7 @@ public function testFetchFile()
4147

4248
public function testFetchFileExcessiveSlashesAreIgnored()
4349
{
44-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co'))->will($this->returnValue(Promise\reject()));
50+
$this->expectRequest($this->uri . 'README.md?view=co')->will($this->returnValue(Promise\reject()));
4551

4652
$promise = $this->client->fetchFile('/README.md');
4753

@@ -50,7 +56,7 @@ public function testFetchFileExcessiveSlashesAreIgnored()
5056

5157
public function testFetchFileRevision()
5258
{
53-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co&pathrev=1.0'))->will($this->returnValue(Promise\reject()));
59+
$this->expectRequest($this->uri . 'README.md?view=co&pathrev=1.0')->will($this->returnValue(Promise\reject()));
5460

5561
$promise = $this->client->fetchFile('/README.md', '1.0');
5662

@@ -59,7 +65,7 @@ public function testFetchFileRevision()
5965

6066
public function testFetchDirectoryRevision()
6167
{
62-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?pathrev=1.0'))->will($this->returnValue(Promise\reject()));
68+
$this->expectRequest($this->uri . 'directory/?pathrev=1.0')->will($this->returnValue(Promise\reject()));
6369

6470
$promise = $this->client->fetchDirectory('/directory/', '1.0');
6571

@@ -68,7 +74,7 @@ public function testFetchDirectoryRevision()
6874

6975
public function testFetchDirectoryAttic()
7076
{
71-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?hideattic=0'))->will($this->returnValue(Promise\reject()));
77+
$this->expectRequest($this->uri . 'directory/?hideattic=0')->will($this->returnValue(Promise\reject()));
7278

7379
$promise = $this->client->fetchDirectory('/directory/', null, true);
7480

@@ -77,7 +83,7 @@ public function testFetchDirectoryAttic()
7783

7884
public function testFetchDirectoryRevisionAttic()
7985
{
80-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?pathrev=1.1&hideattic=0'))->will($this->returnValue(Promise\reject()));
86+
$this->expectRequest($this->uri . 'directory/?pathrev=1.1&hideattic=0')->will($this->returnValue(Promise\reject()));
8187

8288
$promise = $this->client->fetchDirectory('/directory/', '1.1', true);
8389

@@ -86,7 +92,7 @@ public function testFetchDirectoryRevisionAttic()
8692

8793
public function testFetchLogRevision()
8894
{
89-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=log&pathrev=1.0'))->will($this->returnValue(Promise\reject()));
95+
$this->expectRequest($this->uri . 'README.md?view=log&pathrev=1.0')->will($this->returnValue(Promise\reject()));
9096

9197
$promise = $this->client->fetchLog('/README.md', '1.0');
9298

@@ -95,10 +101,22 @@ public function testFetchLogRevision()
95101

96102
public function testFetchPatch()
97103
{
98-
$this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=patch&r1=1.0&r2=1.1'))->will($this->returnValue(Promise\reject()));
104+
$this->expectRequest($this->uri . 'README.md?view=patch&r1=1.0&r2=1.1')->will($this->returnValue(Promise\reject()));
99105

100106
$promise = $this->client->fetchPatch('/README.md', '1.0', '1.1');
101107

102108
$this->expectPromiseReject($promise);
103109
}
110+
111+
private function expectRequest($uri)
112+
{
113+
$that = $this;
114+
115+
return $this->sender->expects($this->once())->method('send')->with($this->callback(function (Request $request) use ($that, $uri) {
116+
$that->assertEquals('GET', $request->getMethod());
117+
$that->assertEquals($uri, $request->getUri());
118+
119+
return true;
120+
}));
121+
}
104122
}

0 commit comments

Comments
 (0)