Skip to main content ↑ ↓
Topic: Possible bug in old_url / Maybe I just don't know the code well enough. (Read 5117 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

Possible bug in old_url / Maybe I just don't know the code well enough.

In subs.php obExit, should $_SESSION['old_url'] really be updated after and xml request?  Or should strpos($_SERVER['REQUEST_URL'], 'xml') === false be added to the list of checks not to update it?

Is there another variable that says where someone came from to an action/page that doesn't get set to xml requests?

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #1 –

Yep, that should be added indeed.
Also, since there are several places where this old_url is set, wouldn't make more sense to create a function setOldUrl or something similar that does that?
Bugs creator.
Features destroyer.
Template killer.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #2 –

Functions are good.  I finally had to do code edits to change the way permissions for posting and editing posts worked.  action_post calls a checkPermissions function.  Post2 does it all inline.  And jsmodify does it all inline but mostly they are redundant as it puts most restrictions on the database jquery to basically enforce the permissions it later checks.

Would have been great if they all called the permissions function in Messages.subs and if that had a hook to override them.  But I know this is legacy from SMF and will be cleaned up over time.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #3 –

If you find those kind of things, feel free to report anything you find (or just fix them if you want to give it a try, in that case feel free to ask anything it may be not clear ;)), it would help as future reference for future refactoring of the code! ;D
Bugs creator.
Features destroyer.
Template killer.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #4 –

Along the same lines, jsmodify calls functions like isAllowedTo() which checks permissions and if they fail redirects to an error page.  Of course that error page is a standard html page not any kind of response expected over ajax so instead of the user seeing the error it seems like nothing happens.

Note that's different than allowedTo which returns a bool (or equivalent) to the php.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #5 –

Yeah, another piece that is not very well structured.
I think isAllowedTo (or better fatal_error) should know "how" to setup the answer (i.e. html, xml, json).
But that's something for 1.1, along with some kind of standardization of the answers I think?

 emanuele feels this should be tracked in the 1200 issue (beta 1.1 goals).
Bugs creator.
Features destroyer.
Template killer.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #6 –

Tried twice to post.  Post button not working.  Trying quick reply.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #7 –

Odd.  Trying normal post again.  If the functions are  going to live on then yes put on the list to make them output format aware.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #8 –

Bump because it has been lost for too long.
Bugs creator.
Features destroyer.
Template killer.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #9 –

On the OP, do we need to add ;api as well .. not sure if we ever call that without xml though

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #10 –

You are probably right, at least for consistency.
Bugs creator.
Features destroyer.
Template killer.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #11 –

And tracked that as well: https://github.com/elkarte/Elkarte/issues/2000
Bugs creator.
Features destroyer.
Template killer.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #12 –

I think I may have added that ;api as part of 1.0.3  O:-)

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #13 –

I can't see it in my repo (script.js line 614).
Bugs creator.
Features destroyer.
Template killer.

Re: Possible bug in old_url / Maybe I just don't know the code well enough.

Reply #14 –

Not the script but in obexit ... forgot about the script :P