Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

If the referrer url is longer than 255 characters, creating a new play will fail #1418

Open
iturgeon opened this issue Nov 26, 2022 · 0 comments · May be fixed by #1428
Open

If the referrer url is longer than 255 characters, creating a new play will fail #1418

iturgeon opened this issue Nov 26, 2022 · 0 comments · May be fixed by #1428
Labels
Awaiting Review Requires a reviewer to test and review code Beginner Friendly This would be a good issue to cut your teeth on Bug

Comments

@iturgeon
Copy link
Member

I discovered this due to a long referrer url filled with a bunch of get params. When the play is created, it looks up the referrer and inserts it into the database. If it's extra long (longer than the db column limits), the insert will fail.

Suggest just truncating the referrer to 255, though increasing the length of the column is a possibility too.
@iturgeon iturgeon added Bug Beginner Friendly This would be a good issue to cut your teeth on Awaiting Review Requires a reviewer to test and review code labels Nov 26, 2022
iturgeon added a commit to iturgeon/Materia that referenced this issue Nov 27, 2022
@iturgeon
Sakai and Canvas LTI. Handle long referrer urls
47fbaa0 
* overhaul lti test provider to simplify
* simplify lti launch picker code when sending content selection messages to lti/assignment
* fixed issue with starting plays with referrer urls longer than 255 characters fixes ucfopen#1418
* updates oauth validation in fuel by ensuring the request uri is not masked by FuelPHP
iturgeon added a commit to iturgeon/Materia that referenced this issue Dec 27, 2022
@iturgeon
Sakai and Canvas LTI. Handle long referrer urls
7497e07 
* overhaul lti test provider to simplify
* simplify lti launch picker code when sending content selection messages to lti/assignment
* fixed issue with starting plays with referrer urls longer than 255 characters fixes ucfopen#1418
* updates oauth validation in fuel by ensuring the request uri is not masked by FuelPHP
iturgeon added a commit to iturgeon/Materia that referenced this issue Dec 28, 2022
@iturgeon
Sakai and Canvas LTI. Handle long referrer urls
9c8cae9 
* overhaul lti test provider to simplify
* simplify lti launch picker code when sending content selection messages to lti/assignment
* fixed issue with starting plays with referrer urls longer than 255 characters fixes ucfopen#1418
* updates oauth validation in fuel by ensuring the request uri is not masked by FuelPHP
iturgeon added a commit to iturgeon/Materia that referenced this issue Dec 31, 2022
@iturgeon
LTI overhaul to support Sakai 22.x
702747b 
* Change var to const for js constants being injected into the page from the server
* Allows the before_play_start event to determine if the lti content item picker should be displayed
* Move picker display function into a common controller
* Add an api method that’ll validate and sign a content item selection lti request
* injects LTI_MESSAGE_TYPE and LTI_KEY js constants into the picker page template
* LTILaunch gains a method to load the lti config that is associated with a lti key
* Updated a few strings that specifically mention canvas
* Moves all classes, tests, migrations, and configs from fuel/app/modules/lti into fuel/app
* overhaul lti test provider to simplify
* simplify lti launch picker code when sending content selection messages to lti/assignment
* fixed issue with starting plays with referrer urls longer than 255 characters fixes ucfopen#1418
* updates oauth validation in fuel by ensuring the request uri is not masked by FuelPHP
* Overhaul lit test provider a bit - mostly to simplify and use lessons
* fix code sniff errors
* add deps to docker image
* attempt to fix yarn cache issue
* Changes docker build process to ensure the built image has admin:make_paths_writable run to avoid having to run it every startup
@iturgeon iturgeon linked a pull request Dec 31, 2022 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Awaiting Review Requires a reviewer to test and review code Beginner Friendly This would be a good issue to cut your teeth on Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Sakai LTI compatibility. LTI module combined with app. iturgeon/Materia
1 participant
@iturgeon