Code

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#3651 closed (fixed)

I18N set_language goes against the recommendations in the HTTP/1.1 specification

Reported by: Fraser Nevett <mail@…> Owned by: nobody
Component: Internationalization Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

From Section 9.1.1 of the HTTP/1.1 specification (RFC 2616): "GET and HEAD methods SHOULD NOT have the significance of taking an action other than retrieval".

In the I18N code, the set_language view allows a user to change their language preference via a GET request. This sets their preference for at least the remainder of their visit to the site, so is doing more than just retrieval.

I know after the GWA content pre-fetching issues that there was some debate (see 1, 2, 3 for a small sample) over the interpretation of the RFCs; however, I would suggest that to comply with the recommendations of the HTTP specification, this method should either:

  1. only accept POST requests, or
  2. require confirmation via a POST request if GET is used.

As the second would require an additional page, I would think the first option is preferable, despite it being a backward incompatible change.

Attachments (1)

set_language_patch.diff (3.0 KB) - added by Fraser Nevett <mail@…> 7 years ago.
Patch for "option 1" (including updates for the documentation)

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Fraser Nevett <mail@…>

Patch for "option 1" (including updates for the documentation)

comment:1 Changed 7 years ago by Fraser Nevett <mail@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Note that the patch also includes Jorge Gajon's patch of #3640.

comment:2 Changed 7 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

Agree that this is a bug. It's also a backwards incomaptible change, at least for those people using the GET method to call this view at the moment. So we'll apply the fix after 0.96 is released.

comment:4 Changed 7 years ago by Simon G. <dev@…>

  • Triage Stage changed from Accepted to Ready for checkin

0.96 is out, so moving back to RFC

comment:5 Changed 7 years ago by mtredinnick

  • Owner changed from hugo to mtredinnick

It's just a matter of timing on this (the ready for checkin status wasn't affecting anything). I'm trying to pick the moment to break a lot of i18n'd applications. I'm trying to roll a bunch of backwards-incompatible changes into one short period so that people don't spend their entire lives forward porting.

This is on my list and will go in when I have some other significant breakage to commit.

comment:6 Changed 7 years ago by Simon G. <dev@…>

As always, Malcolm, you're way ahead of me!

comment:7 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [6177]) Fixed #3651 -- Changed set_language_view() to require POST request is used, in accordance with the HTTP spec (it changes the user's state). Thanks, Fraser Nevett.

This is a backwards incompatible change for anybody previously using this view.

comment:8 Changed 7 years ago by Marc Fargas <telenieko@…>

Although the ticket is already closed, I didn't spot it until now, sorry.

I just have a few questions about the banckwards incompatible change:

  • Sites like to have language links somewhere (like flags) that actually point to the GET URL to change language, they'd now require an additional page to change languages via POST.
  • At least me, on google adwords I put links using the GET thing with the &next= parameter to ensure people clicking adds see the pages on the same language they saw the ad.

Maybe there are some other cases that you need to change languages and cannot use POST, but I can only thing about those two now.

So, while being RFC compliant could we provide a "GET aware" solution? maybe checking on the view if parameters where passed via GET and then render a confirmation form for the change that submits also to the view via POST ? This option would also keep things a bit more backwards complatible for current users of the GET method (given they provide a template for the confirmation form).

Also, could a parameter be given to the GET to "skip the confirmation form" for cases like the second one I said?

The first point is what the ticket describes as Option 2, it should be easy to implement if you feel ok with it. The second idea means "allow the user/developer break RFC if he/she needs so".

What do you think of both ideas?

comment:9 Changed 6 years ago by jcassee

Would Malcolm (mtredinnick) please review a similar situation with the logout view at ticket #7989? Thanks!

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.