[Request] Added a getBaseServerUrl() function #3416

Closed
wants to merge 6 commits into
from

Projects

None yet

5 participants

@COil

Here is a use case for this little PR. I wanted to have the full URL for an image related to an object (relative URL is stored into the 'webPath' property of the 'job' object)

<div>
    <a href="{{ job.url }}">
        <img src="{{ app.request.scheme ~ '://' ~ app.request.host }}{{ asset(job.webPath) }}" alt="{{ job.company }} logo" />
    </a>
</div>

This twig code is quiet complicated just to retrieve the base URi... so with the PR the code turns into:

<div>
    <a href="{{ job.url }}">
        <img src="{{ app.request.getBaseUri }}{{ asset(job.webPath) }}" alt="{{ job.company }} logo" />
    </a>
</div>

Which is far more compact. Additional unit tests are included.

COil.

@stof stof commented on an outdated diff Feb 22, 2012
src/Symfony/Component/HttpFoundation/Request.php
@@ -744,7 +744,19 @@ public function getUri()
*/
public function getUriForPath($path)
{
- return $this->getScheme().'://'.$this->getHttpHost().$this->getBaseUrl().$path;
+ return $this->getBaseUri().$this->getBaseUrl().$path;
+ }
+
+ /**
+ * Generates the base URI.
+ *
+ * @return string The URI for the base public path
+ *
+ * @api
+ */
+ public function getBaseUri()
+ {
+ return $this->getScheme().'://'.$this->getHttpHost();
@stof
stof Feb 22, 2012

This is not the base uri. The base uri should include the base path necessary to reach your web root

@COil

Humm, you are right Stof ! I knew I add forgotten something... :x Well in fact, getUriForPath('') is what I want, It works in prod env (without the controller) but it's not correct when the controller is used in the URL. So what is the easiest way to handle this ? Compute this path after prepareBaseUrl() ?

Other user who add a similar question:
http://groups.google.com/group/symfony2/browse_thread/thread/6a6f14ad88e3fc0?pli=1

@stof
Symfony member

Well, getUriForPath is meant to give the uri for a path inside the app, so it is logical to include the front controller in it.

@COil

So, how to write the following code ?

<div>
    <a href="{{ job.url }}">
        <img src="{{ app.request.scheme ~ '://' ~ app.request.host }}{{ asset(job.webPath) }}" alt="{{ job.company }} logo" />
    </a>
</div>

It's wrong too if the root of the web application is not /.

@stof
Symfony member

@COil no it is not, as the asset function is precisely meant to deal with it

@COil

Yes, you are right. So the name of my function is wrong, it should be something like getBaseServerUrl()

@dlsniper

Hi, any news about this?

@lsmith77 lsmith77 and 1 other commented on an outdated diff Mar 25, 2012
README.md
@@ -1,7 +1,7 @@
README
======
-[![Build Status](https://secure.travis-ci.org/symfony/symfony.png?branch=master)](http://travis-ci.org/symfony/symfony)
+[![Build Status](https://secure.travis-ci.org/COil/symfony.png?branch=master)](http://travis-ci.org/COil/symfony)
@lsmith77
lsmith77 Mar 25, 2012

that needs to be reverted

@stof
Symfony member

@COil Please rebase your branch. It conflicts with master because of the move of tests. You should probably squash the commits too.

@fabpot what do you think about it ?

@dinamic

Why not an optional parameter to the asset() twig function?

@COil

@dinamic Indeed, it could be an improvement from the Twig side

@stof I have created a new clean PR #4312

@stof stof closed this May 17, 2012
@fabpot fabpot added a commit to fabpot/symfony that referenced this pull request Jun 28, 2012
@fabpot fabpot added Request::getSchemeAndHttpHost() and Request::getUserInfo() (clo…
…ses #4312, refs #3416, refs #3056)
df8d94e
@fabpot fabpot added a commit that referenced this pull request Jun 28, 2012
@fabpot fabpot merged branch fabpot/request-methods (PR #4679)
Commits
-------

df8d94e added Request::getSchemeAndHttpHost() and Request::getUserInfo() (closes #4312, refs #3416, refs #3056)

Discussion
----------

added Request::getSchemeAndHttpHost() and Request::getUserInfo()

see #4312

---------------------------------------------------------------------------

by travisbot at 2012-06-28T15:22:03Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730172) (merged 598bd56f into 0d27570).

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:22:35Z

Why not just `getSchemeAndHost`? That sounds long enough, and is fairly explicit given the context.

---------------------------------------------------------------------------

by fabpot at 2012-06-28T15:25:34Z

@Seldaek because (and that's probably unfortunate) we have both `getHost()` and `getHttpHost()`. The former does not include the port whereas the latter does.

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:26:42Z

Ok makes sense.

---------------------------------------------------------------------------

by travisbot at 2012-06-28T16:11:16Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730630) (merged df8d94e into 884a826).
e0351c9
@mmucklo mmucklo pushed a commit that referenced this pull request May 23, 2013
@fabpot fabpot merged branch fabpot/request-methods (PR #4679)
Commits
-------

df8d94e added Request::getSchemeAndHttpHost() and Request::getUserInfo() (closes #4312, refs #3416, refs #3056)

Discussion
----------

added Request::getSchemeAndHttpHost() and Request::getUserInfo()

see #4312

---------------------------------------------------------------------------

by travisbot at 2012-06-28T15:22:03Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730172) (merged 598bd56f into 0d27570).

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:22:35Z

Why not just `getSchemeAndHost`? That sounds long enough, and is fairly explicit given the context.

---------------------------------------------------------------------------

by fabpot at 2012-06-28T15:25:34Z

@Seldaek because (and that's probably unfortunate) we have both `getHost()` and `getHttpHost()`. The former does not include the port whereas the latter does.

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:26:42Z

Ok makes sense.

---------------------------------------------------------------------------

by travisbot at 2012-06-28T16:11:16Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730630) (merged df8d94e into 884a826).
8dad8dc
@hackzilla hackzilla pushed a commit that referenced this pull request Dec 10, 2014
@fabpot fabpot merged branch fabpot/request-methods (PR #4679)
Commits
-------

df8d94e added Request::getSchemeAndHttpHost() and Request::getUserInfo() (closes #4312, refs #3416, refs #3056)

Discussion
----------

added Request::getSchemeAndHttpHost() and Request::getUserInfo()

see #4312

---------------------------------------------------------------------------

by travisbot at 2012-06-28T15:22:03Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730172) (merged 598bd56f into 0d27570).

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:22:35Z

Why not just `getSchemeAndHost`? That sounds long enough, and is fairly explicit given the context.

---------------------------------------------------------------------------

by fabpot at 2012-06-28T15:25:34Z

@Seldaek because (and that's probably unfortunate) we have both `getHost()` and `getHttpHost()`. The former does not include the port whereas the latter does.

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:26:42Z

Ok makes sense.

---------------------------------------------------------------------------

by travisbot at 2012-06-28T16:11:16Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730630) (merged df8d94e into 884a826).
d653bbd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment