If you're using a local development environment eg. local.dev:8888 then we only want "local.dev" to get sent. Patch included.

Files: 
CommentFileSizeAuthor
#8 fontyourface-Make_Fontdeck_parse_the_domain_correctly-1611902-8.patch1.79 KBDrave Robber
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]
#3 fontyourface-Make_Fontdeck_parse_the_domain_correctly-1611902-3.patch1.54 KBDrave Robber
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]
fontdeck.patch1.36 KBaidanlis
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fontdeck.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

sreynen’s picture

Version:6.x-2.x-dev» 7.x-2.x-dev
Status:Active» Needs work

Thanks for the report and the patch. A few issues:

1) Let's fix the 7.x branch first, then backport.
2) Patches should be made following the instructions at http://drupal.org/patch/create
3) Rather than setting the value of $domain, then returning, I think we can skip a step and return the result of the preg_replace() directly.

aidanlis’s picture

I don't have time to play patch tennis, so do with it what you will :)

Drave Robber’s picture

Title:Fontdeck does not parse the domain correctly» Make Fontdeck parse the domain correctly
Component:Code (general)» Fontdeck (provider)
Status:Needs work» Needs review
StatusFileSize
new1.54 KB
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]

Here's it against 7.x-2.x-dev, with the following changes:

  • implemented sreynen's 3) of #1,
  • let's call host a host.
sreynen’s picture

Status:Needs review» Needs work

This isn't working for me. Specifically, parse_url($_SERVER['HTTP_HOST'], PHP_URL_HOST) is returning FALSE, I think because $_SERVER['HTTP_HOST'] is only host, not a full URL. $host = parse_url('http://' . $_SERVER['HTTP_HOST'], PHP_URL_HOST); works for me. That feels a bit hackish, but I don't have any better ideas.

Drave Robber’s picture

According to docs,

Partial URLs are also accepted, parse_url() tries its best to parse them correctly.

...but sometimes the best is just not enough.
As the whole purpose of this was to strip port numbers, it could as well read:
$host = parse_url($_SERVER['HTTP_HOST'], PHP_URL_PORT) > 0 ? parse_url($_SERVER['HTTP_HOST'], PHP_URL_HOST) : $_SERVER['HTTP_HOST'];
I did a couple of quick tests, strings like localhost:666 and 127.0.0.1:666 are parsed correctly.

Drave Robber’s picture

Or, for slightly shorter lines and to avoid calling parse_url() twice:

<?php
$details
= parse_url($_SERVER['HTTP_HOST']);
$host = isset($details['port']) ? $details['host'] : $_SERVER['HTTP_HOST'];
?>
sreynen’s picture

I did some more testing to try to figure out where parse_url() breaks down. It looks like it works consistently with a host and a port, but always fails with just the host. So I think #6 will work, though I'd change the isset($details['port']) to isset($details['host']). If it somehow did parse the host without a port, I think we'd still want the parsed host.

Drave Robber’s picture

Status:Needs work» Needs review
StatusFileSize
new1.79 KB
PASSED: [[SimpleTest]]: [MySQL] 9 pass(es).
[ View ]

Implements #6 + #7, plus adds a somewhat verbose comment on what all this means.

sreynen’s picture

Status:Needs review» Fixed
Drave Robber’s picture

Version:7.x-2.x-dev» 6.x-2.x-dev
Status:Fixed» Patch (to be ported)

#8 needs backporting to D6.

sreynen’s picture

Issue tags:+Novice

Tagging.

Drave Robber’s picture

Status:Patch (to be ported)» Postponed