Project

General

Profile

Bug #8889

get_variadic_url oublie la query string pour [site-url]?var=value

Added by Thomas Noël almost 7 years ago. Updated almost 7 years ago.

Status:
Fermé
Priority:
Normal
Assignee:
Target version:
Start date:
05 November 2015
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:

Description

Un test qui fait planter :

diff --git a/tests/test_variadic_url.py b/tests/test_variadic_url.py
index 447bbfd..f8f4dd0 100644
--- a/tests/test_variadic_url.py
+++ b/tests/test_variadic_url.py
@@ -64,6 +64,10 @@ def test_url_server():
     assert get_variadic_url('[url]/foobar',
             {'url': 'http://www.example.net'}) == 'http://www.example.net/foobar'

+def test_url_server_qs_without_slash():
+    assert get_variadic_url('[url]?foo=bar',
+            {'url': 'http://www.example.net/'}) == 'http://www.example.net/?foo=bar'
+
 def test_url_server_more():
     assert get_variadic_url('[url]/foobar/json?toto',
             {'url': 'http://www.example.net'}) == 'http://www.example.net/foobar/json?toto'

donne :

E       assert 'http://www.example.net/' == 'http://www.example.net/?foo=bar'
E         - http://www.example.net/
E         + http://www.example.net/?foo=bar
E         ?                        ++++++++

J'en suis à regarder et à penser que ça se joue ici :

323                 if p2.path and not path:
324                     path, query = p2.path, p2.query

où on oublie de penser qu'il y a peut-être une query existante, qu'on oublie gentilment (foo=bar dans le test)


Files

0001-fix-in-variadic-URL-8889.patch (1.74 KB) 0001-fix-in-variadic-URL-8889.patch Thomas Noël, 06 November 2015 12:04 AM
0001-fix-in-variadic-URL-8889.patch (2.37 KB) 0001-fix-in-variadic-URL-8889.patch Thomas Noël, 06 November 2015 12:12 AM
0001-misc-extend-variadic-URL-to-support-even-more-variat.patch (4.69 KB) 0001-misc-extend-variadic-URL-to-support-even-more-variat.patch Frédéric Péters (de retour le 10/10), 06 November 2015 10:20 AM

Associated revisions

Revision 143d4329 (diff)
Added by Thomas Noël almost 7 years ago

misc: don't lose query string in variadic URL (#8889)

Revision 6cf22249 (diff)
Added by Frédéric Péters (de retour le 10/10) almost 7 years ago

misc: extend variadic URL to support even more variations (#8889)

History

#1

Updated by Thomas Noël almost 7 years ago

  • Status changed from Nouveau to En cours
  • Patch proposed changed from No to Yes

Pas très joli mais à cette heure j'ai rien d'autre à proposer.

#3

Updated by Thomas Noël almost 7 years ago

et ça permet d'autres magies qui pourraient être imaginées, j'ajoute un test.

#4

Updated by Thomas Noël almost 7 years ago

posé vite sur https://demarches-alfortville.test.entrouvert.org/

(mais une autre correction sera possible)

#5

Updated by Thomas Noël almost 7 years ago

  • Subject changed from get_variadic_url to get_variadic_url oublie la query string pour [site-url]?var=value
  • Assignee set to Thomas Noël
#6

Updated by Frédéric Péters (de retour le 10/10) almost 7 years ago

Je voudrais bien voir les tests doublés (ça peut être simplement en dupliquant la ligne asset), une fois avec 'url': 'http://www.example.net/' et une fois avec 'url': 'http://www.example.net'.

#7

Updated by Frédéric Péters (de retour le 10/10) almost 7 years ago

Voilà un second patch, avec des tests supplémentaires.

Pour le premier c'est ok mais il lui faut un autre sujet, "misc: don't lose query string in variadic URL (#8889)".

#8

Updated by Thomas Noël almost 7 years ago

  • Status changed from En cours to Résolu (à déployer)
  • Target version set to v1.22
commit 6cf22249aa3da06ab5d26164a403be548f719bdf
Author: Frédéric Péters <fpeters@entrouvert.com>
Date:   Fri Nov 6 10:20:10 2015 +0100

    misc: extend variadic URL to support even more variations (#8889)

commit 143d432905140feeb908df235f62e48f2bef56a9
Author: Thomas NOEL <tnoel@entrouvert.com>
Date:   Fri Nov 6 00:03:37 2015 +0100

    misc: don't lose query string in variadic URL (#8889)

#9

Updated by Frédéric Péters (de retour le 10/10) almost 7 years ago

  • Status changed from Résolu (à déployer) to Fermé

Also available in: Atom PDF