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

[ticket/12787] Allow the route to say that the referer has to be used. #2676

Merged
merged 9 commits into from Jul 8, 2014

Conversation

Nicofuma
Copy link
Member

@bantu
Copy link
Collaborator

bantu commented Jun 29, 2014

There has been a previous patch using referer. The referer is not an acceptable solution as it might just be off.

@@ -111,6 +119,33 @@ public function getController(Request $request)
}
}

// If the request is made by ajax and if requested, fix the request_uri to use the referer.
// If the request has an _referer attribute it is used, otherwise we use the request header if available.
if ($request->isXmlHttpRequest() && $request->get('_ajax_fix_request_uri', false) && $request instanceof \phpbb\symfony_request)
Copy link
Contributor

Choose a reason for hiding this comment

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

$request->is_ajax()

@Nicofuma
Copy link
Member Author

i didn't find another solution because we need the real request_uri for the route matcher
the only way to solved this issue is to dynamically change the request_uri...
but it can't be calculated by the server, so we have to trust the user input for that

and the easiest way to get the value is to use the referer (here as a fallback if the referer isn't specified as argument in the query). But we could remove this fallback yes...

@bantu
Copy link
Collaborator

bantu commented Jun 29, 2014

Please dig up the other PR.

@nickvergessen
Copy link
Contributor

PR is useless as I rebased, here is the branch before rebase:
https://github.com/nickvergessen/phpbb/compare/ticket/12099-ajax-referer

Pico and others added 6 commits June 29, 2014 20:54
If the current request is a AJAX we need to fix the paths.
We need to get the root path based on the Referer, so we can use
the generated URLs in the template of the Referer. If we do not
generate the relative path based on the Referer, but based on the
currently requested URL, the generated URLs will not point to the
intended locations:
	Referer				desired URL			desired relative root path
	memberlist.php		faq.php				./
	memberlist.php		app.php/foo/bar		./
	app.php/foo			memberlist.php		../
	app.php/foo			app.php/fox			../
	app.php/foo/bar		memberlist.php		../../
	../page.php			memberlist.php		./phpBB/
	../sub/page.php		memberlist.php		./../phpBB/

PHPBB3-12099
@bantu
Copy link
Collaborator

bantu commented Jun 29, 2014

@nickvergessen I think there was a description of a proper solution in that PR.

@nickvergessen
Copy link
Contributor

@bantu if by proper you mean "this is unacceptable" then yes, otherwise nothing usable was supplied: #2427

@bantu
Copy link
Collaborator

bantu commented Jun 29, 2014

@nickvergessen There you go. #2427 (comment)

@Nicofuma
Copy link
Member Author

about this i think that it should be an other PR (a generalization of this one which allows a request to define the refer to use)

@Nicofuma
Copy link
Member Author

Nicofuma commented Jul 4, 2014

bump

@nickvergessen
Copy link
Contributor

You have an ext ready for testing?

@Nicofuma
Copy link
Member Author

Nicofuma commented Jul 4, 2014

only the profiler which needs another PRs... (the links in the toolbar: http://phpbb3-1.nicofuma.fr/upstream/demo/world)

@Nicofuma
Copy link
Member Author

Nicofuma commented Jul 4, 2014

The URL which use the referer is in the ajax call at the end of the HTML code and this ajax call generate the toolbar (with the links)

@Nicofuma
Copy link
Member Author

Nicofuma commented Jul 5, 2014

updated

@nickvergessen
Copy link
Contributor

Created a new branch in the acme/demo which tests ranks, smilies (put one in your signature) and normal links:
https://github.com/nickvergessen/phpbb-ext-acme-demo/tree/testing-webrootpath

@nickvergessen
Copy link
Contributor

generate_board_url(true) . $this->symfony_request->getRequestUri() as _referer works.
We should just make that an easy function in the controller_helper

@nickvergessen
Copy link
Contributor

Results of the app should be verified on:

  • phpBB/index.php
  • phpBB/web/root/test
  • phpBB/app.php/web/root/test

nickvergessen added a commit to nickvergessen/phpbb that referenced this pull request Jul 8, 2014
[ticket/12787] Allow the route to say that the referer has to be used.

* Nicofuma/ticket/12787:
  [ticket/12787] Updates phpbb_mock_controller_helper
  [ticket/12787] Add controller_helper::get_current_url()
  [ticket/12787] Remove one app.php when it's both in $path and $web_root_path
  [ticket/12787] Fix the absolute board url
  [ticket/12787] Use a parameter (_referer) instead of the Referer header
  [ticket/12099] Add unit tests for get_web_root_path_from_ajax_referer()
  [ticket/12099] Remove config again
  [ticket/12099] Correctly fix the path when performing AJAX requests
  [ticket/12099] Add request argument to path_helper service
@nickvergessen nickvergessen merged commit b4d7192 into phpbb:develop-ascraeus Jul 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants