Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #66 #67

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix issue #66 #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
916a8e1
Fix issue #66
peter279k Jun 24, 2018
bcb853c
Rebuild the hashbang with colon
peter279k Jun 25, 2018
dfd5966
Merge branch 'master' into issues_#66
peter279k Sep 30, 2018
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
14 changes: 13 additions & 1 deletion src/Purl/Fragment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use function array_merge;
use function parse_url;
use function sprintf;
use function strpos;
use function substr;

/**
* Fragment represents the part of a Url after the hashmark (#).
Expand Down Expand Up @@ -113,7 +115,17 @@ public function __toString() : string
protected function doInitialize() : void
{
if ($this->fragment !== null) {
$this->data = array_merge($this->data, parse_url($this->fragment));
$pos = strpos($this->fragment, ':', 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is my approach:

  • Detect the hash bang whether it's with the colon.
  • Rebuild the hash bang with the colon after using the parse_url function.

Copy link
Contributor

Choose a reason for hiding this comment

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

@peter279k: Are we sure that this is the only case that would cause parse_str to fail ? Why not just use parse_str right away, and fallback to $data = ['path' => $this->fragment] if it fails ?

It would make less assumptions about how parse_str works now and in the future. This would make this code more robust.

$colon = '';
if ($pos !== false) {
$colon = substr($this->fragment, $pos);
}
$data = parse_url($this->fragment);
if ($colon !== '') {
$data = ['path' => $this->fragment];
}

$this->data = array_merge($this->data, $data);
}

foreach ($this->data as $key => $value) {
Expand Down
4 changes: 4 additions & 0 deletions tests/Purl/Test/UrlTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,10 @@ public function testRelativeUrl() : void
$url = new Url('/events');
$url->query->set('param1', 'value1');
$this->assertEquals('/events?param1=value1', (string) $url);

// test fragment with colon
$url = new Url('http://example.com/#hello:123');
$this->assertEquals('http://example.com/#hello:123', (string) $url);
}
}

Expand Down