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.

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

$baseUrl not using correct port with Vagrant virtual machine #325

Closed
richardkmiller opened this issue Jul 4, 2014 · 13 comments
Closed

$baseUrl not using correct port with Vagrant virtual machine #325

richardkmiller opened this issue Jul 4, 2014 · 13 comments

Comments

@richardkmiller
Copy link

Related to #175. When using Restler in development with a Vagrant VM, I'm using port 80 on the inside of the VM and port 8088 on the outside (port forwarded). Because $_SERVER['SERVER_PORT'] is set to 80 on the inside of the VM, Restler sets $baseUrl to localhost instead of the correct localhost:8088.

Solutions:

  1. A new setter method setBaseUrl() so I can set $baseUrl manually.
  2. Restler's getPath() could use $_SERVER['HTTP_HOST'] instead of $_SERVER['SERVER_NAME'] + $_SERVER['SERVER_PORT'].
  3. I could alter $_SERVER['SERVER_PORT'] before Restler runs, as suggested in Incorrect $baseUrl problem & fix #175 for $_SERVER['SERVER_NAME'], but that seems problematic as it might break something else?

Thanks!

@richardkmiller
Copy link
Author

Btw, glad to submit a PR for 1 or 2 if you like.

Arul- added a commit that referenced this issue Jul 6, 2014
… url and multi base url configurations. Closes #325
@Arul-
Copy link
Member

Arul- commented Jul 6, 2014

You have made a very valid point @richardkmiller 👍 I have taken solution 1 and improvised on it :)

You can add one or more baseUrls, when you add more than one, restler will pick the right one based on $_SERVER['HTTP_HOST']

in your case the following should make it work inside and outside vagrant

$r = new Restler();
$r->setBaseUrl('http://localhost','http://localhost:8088');
//...

The above commit is in latest v3 branch and will be part of RC6 when released.

Please test and let me know!

We welcome your contributions to Restler 👍

@richardkmiller
Copy link
Author

@Arul- Thanks! That will work great for our needs!

However, I am currently getting a warning on this branch:
PHP Warning: strpos(): Empty needle in /vagrant/vendor/luracast/restler/vendor/Luracast/Restler/Restler.php on line 482

I tried $r->setBaseUrl('http://localhost', 'http://localhost:8088');
and just $r->setBaseUrl('http://localhost:8088');

The call to Util::splitCommonPath in 463 is returning empty. Here are the parameters:

$_SERVER['REQUEST_URI']: /resources.json
$_SERVER['SCRIPT_NAME']: /index.php

Arul- added a commit that referenced this issue Jul 8, 2014
… url and multi base url configurations. Closes #325
@Arul-
Copy link
Member

Arul- commented Jul 8, 2014

It should be fixed now, check it out!

@richardkmiller
Copy link
Author

Interestingly, it works when I list my preferred hostname last:
$r->setBaseUrl('http://localhost', 'http://localhost:8088');

But not if I list it first:
$r->setBaseUrl('http://localhost:8088', 'http://localhost');

Arul- added a commit that referenced this issue Jul 9, 2014
@Arul-
Copy link
Member

Arul- commented Jul 9, 2014

Taken care of the issue! Please check again!

@richardkmiller
Copy link
Author

Well, it worked in development but not in production. Here's my full configuration including production:

$r->setBaseUrl('http://localhost:8088', 'http://localhost', 'http://services1', 'http://services2', 'http://services3', 'http://services4', 'http://services5', 'http://services');

There are 5 services machines running this code, named services1-5. There's also a round-robin DNS entry "services" that points to the 5 services machines. If you visit http://services, your browser will choose one of the five machines. I'm seeing erratic behavior. For example, if I visit http://services1/resources.json, it returns basePath http://services3. Maybe it's something with strpos() === 0 because all of these hostnames begin with "services" and then usort() causes the order to be unpredictable (it's unpredictable but consistent -- the basePath is always http://services3).

@Arul-
Copy link
Member

Arul- commented Jul 10, 2014

It seems to work correctly, see my test below

<?php
require '../vendor/autoload.php';

use Luracast\Restler\Restler;

$r = new Restler();

$_SERVER['HTTP_HOST'] = 'http://services1';

$r->setBaseUrl(
    'http://localhost:8088', 
    'http://localhost', 
    'http://services1', 
    'http://services2', 
    'http://services3', 
    'http://services4', 
    'http://services5', 
    'http://services'
);

dd($r->getBaseUrl());

I changed the HTTP_HOST to different values and I can see that the same host is returned

usort is only sorting by string length and that wont cause any issue!

@richardkmiller
Copy link
Author

@Arul- Is the $_SERVER['HTTP_HOST'] statement required? Correct me if I'm wrong; I thought your new setBaseUrl code was meant to remove the need to manually set HTTP_HOST? For example, we deploy the same configuration file to every services machine. It would require new deployment code to set HTTP_HOST to a different value on each machine.

I think you're correct that usort isn't a problem per se. It simply creates what looks like a random change from services1 to services. (It's not in fact random; it's simply the internals of the usort algorithm and it is consistent.)

Could this be solved by doing an equality match == instead of a strpos() === 0 match? That's my best guess as to what is happening.

@richardkmiller
Copy link
Author

@Arul- Thank you for your help with this!

@Arul-
Copy link
Member

Arul- commented Jul 10, 2014

The HTTP_HOST statement is only for simulating your environment :)

as my $_SERVER['HTTP_HOST'] won't match yours!

@Arul-
Copy link
Member

Arul- commented Jul 10, 2014

I also made sure to select the first url when none of them match, in the recent commit!

@Arul-
Copy link
Member

Arul- commented Jul 22, 2014

I assume this issue is fixed as I did not hear back from you so far!

Please re-open the issue if its not!

@Arul- Arul- closed this as completed Jul 22, 2014
Arul- added a commit that referenced this issue Jul 22, 2014
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Jul 22, 2014
Arul- added a commit that referenced this issue Aug 10, 2014
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Aug 10, 2014
Arul- added a commit that referenced this issue Aug 11, 2014
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Aug 11, 2014
Arul- added a commit that referenced this issue Sep 3, 2014
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Sep 3, 2014
Arul- added a commit that referenced this issue Dec 8, 2014
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Dec 8, 2014
Arul- added a commit that referenced this issue Jun 7, 2015
…e given list. Also renamed the method to setBaseUrls to better convey the purpose. Related to #325
Arul- added a commit that referenced this issue Jun 12, 2015
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Jun 12, 2015
Arul- added a commit that referenced this issue Jun 12, 2015
…e given list and adds support for specifying the http port. Also renamed the method to setBaseUrls to better convey the purpose. Related to #325
Arul- added a commit that referenced this issue Jul 19, 2015
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Jul 19, 2015
Arul- added a commit that referenced this issue Jul 19, 2015
…e given list and adds support for specifying the http port. Also renamed the method to setBaseUrls to better convey the purpose. Related to #325
Arul- added a commit that referenced this issue Aug 4, 2015
… url and multi base url configurations. Closes #325
Arul- added a commit that referenced this issue Aug 4, 2015
Arul- added a commit that referenced this issue Aug 4, 2015
…e given list and adds support for specifying the http port. Also renamed the method to setBaseUrls to better convey the purpose. Related to #325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants