https://bugzilla.wikimedia.org/show_bug.cgi?id=43360
--- Comment #9 from Daniel Friesen <[email protected]> --- (In reply to comment #7) > Daniel (and Laxstrom) > > I totally agree about the ugly hack - and if there are suggestions on > improvement, please submit. HOWEVER, if you look at rest of the code as it > relates to button in createFormLink: > > 1. If I'm reading the code right, the other query params are already being > handled correctly by adding them as hidden values 3 lines subsequently. Those are query parameters added by things like |query string= inside the form definition. Any other query params inside link_url are not being handled. > 2. The only query that isn't being handled by this is the "title" param > and this hack handles that (and only that) exception. > 3. The "title" param is returned as part of the getTitle()->getLocalURL(). > If there is a way to get ONLY the ugly "title" param, it would probably > be more appropriate (I'm just not aware) getLocalURL is hookable. Other extensions may make it return urls with other query params and in a different order. Though just for the record, you can get the same title text out of Title that getLocalURL uses to make the title param. > 4. The hack leaves the "title=" in the URL, which is ignored, but confusing. > Is it worth removing? Is there a possible security implication in leaving > it visible? Should it be removed? Removing the params that have been moved into inputs is the correct thing to do. > 5. As ugly as this is, I believe it covers all conditions and solves the > problem. As Yaron stated, the alternative is to use POST, but I'm > assuming there is a reason somewhere (in my case, legacy apps), that > GET is preferred. False dilemma, neither POST nor hacks of this level are necessary. It is not that hard to write some sane code that actually handles this situation correctly without being a hack. - Don't look for ?title. Instead break apart the url and it's query string. - Add hidden inputs for all the params you find in the query string. This can be done in the same way that the code is already doing to handle the user provided |query string= data. In fact you could probably merge the queries and avoid code duplication. - Don't do this inside of the `if ( ! empty( $inQueryArr ) ) {` conditional. Doing this inside of there leads to the possibility of link_url not being properly handled and the bug coming back in cases like extra user specified query strings not being provided. Frankly createFormLink's generation of the $link_url in the first place needs some fixes too. It shouldn't be appending to the url like that. That stuff should go into the title (or rather special page subtitle) before getLocalURL is called. -- You are receiving this mail because: You are on the CC list for the bug. You are watching all bug changes. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
