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.

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

Remove viewsqlite.php and any SQLite leftovers #2823

Merged
merged 2 commits into from
Apr 18, 2023
Merged

Remove viewsqlite.php and any SQLite leftovers #2823

merged 2 commits into from
Apr 18, 2023

Conversation

stoyan
Copy link
Contributor

@stoyan stoyan commented Apr 11, 2023

Quick fixes:

  • isset() was causing a 500
  • looping over null $results was causing an error too

Long term question:

  • Delete this completely?

@stoyan stoyan changed the title Unbreak viewsqlite.php Remove viewsqlite.php and any SQLite leftovers Apr 12, 2023
@stoyan
Copy link
Contributor Author

stoyan commented Apr 12, 2023

Turns out this is a leftover from an old system of labels.

There was one place that was still working: in the compare page there's an option to edit a label.

Screenshot 2023-04-12 at 3 12 26 PM

This label is written to /dat/labels.db

There are few problems with this:

  • privacy: I changed a label of a test I didn't run. It persisted in incognito, so it's changed for everyone
  • security: admittedly a low level but an alert() in the title executed the script while modifying the label. After a refresh it was escaped properly
  • general confusion: the label is changed only on the compare page, the test otherwise shows the original label

Overall I love the idea of changing a label post-factum, but maybe a better implementation is in order

Copy link
Contributor

@pmeenan pmeenan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the cleanup

@stoyan stoyan merged commit 3d43f6b into master Apr 18, 2023
@stoyan stoyan deleted the viewsqlite branch April 18, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants