Description
While looking into the csrf_failure view, I noticed that it tries to extract a Program object from the request path, but the logic doesn’t seem to behave as expected.
The regex used for parsing doesn’t account for the leading / in request.path, so it may fail to match in normal cases. On top of that, even if a program is successfully resolved, it isn’t added to the template context, so it doesn’t actually affect the rendered response.
I also noticed that prog isn’t currently used anywhere in 403_csrf_failure.html, which makes this logic feel a bit incomplete or possibly leftover from an earlier implementation. Would it make sense to pass prog into the template and use it there, or should this lookup be removed entirely if it’s no longer needed?
One thought I had was to simplify the parsing by using request.path.strip('/').split('/'), which is easier to read and maintain. That said, I realize the current regex does provide some basic validation (restricting characters before hitting the database), so it might be preferable to keep the regex and just adjust it to handle the leading / properly.
Overall, just wanted to flag this in case it’s unintended behavior or something that could be cleaned up 👍
Steps to Reproduce
- Trigger a CSRF failure on a URL that includes a program path (e.g. /something/program1/session1/...)
- Observe the behavior inside csrf_failure view
- Check whether:
The regex successfully matches the path
The resolved prog is passed to the template
Expected Behavior
The program should be correctly extracted from the request path
The resolved Program object should be passed into the template context
The CSRF failure page should have access to program-specific data (if needed)
Actual Behavior
The regex may fail due to the leading / in request.path
Even when a program is found, it is not added to the context
The computed prog variable is effectively unused
Screenshots
No response
Operating System
Windows 11
Browser
Chrome
Additional Context
A simple improvement could be:
Replacing regex parsing with request.path.strip('/').split('/')
Adding prog to the context (c['prog'] = prog)
Happy to work on a fix if this approach looks good.
Description
While looking into the
csrf_failureview, I noticed that it tries to extract aProgramobject from the request path, but the logic doesn’t seem to behave as expected.The regex used for parsing doesn’t account for the leading
/inrequest.path, so it may fail to match in normal cases. On top of that, even if a program is successfully resolved, it isn’t added to the template context, so it doesn’t actually affect the rendered response.I also noticed that
progisn’t currently used anywhere in403_csrf_failure.html, which makes this logic feel a bit incomplete or possibly leftover from an earlier implementation. Would it make sense to passproginto the template and use it there, or should this lookup be removed entirely if it’s no longer needed?One thought I had was to simplify the parsing by using
request.path.strip('/').split('/'), which is easier to read and maintain. That said, I realize the current regex does provide some basic validation (restricting characters before hitting the database), so it might be preferable to keep the regex and just adjust it to handle the leading/properly.Overall, just wanted to flag this in case it’s unintended behavior or something that could be cleaned up 👍
Steps to Reproduce
The regex successfully matches the path
The resolved prog is passed to the template
Expected Behavior
The program should be correctly extracted from the request path
The resolved Program object should be passed into the template context
The CSRF failure page should have access to program-specific data (if needed)
Actual Behavior
The regex may fail due to the leading / in request.path
Even when a program is found, it is not added to the context
The computed prog variable is effectively unused
Screenshots
No response
Operating System
Windows 11
Browser
Chrome
Additional Context
A simple improvement could be:
Replacing regex parsing with request.path.strip('/').split('/')
Adding prog to the context (c['prog'] = prog)
Happy to work on a fix if this approach looks good.