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

fixes xenos seeing raw html on paper #4654

Closed
wants to merge 5 commits into from
Closed

fixes xenos seeing raw html on paper #4654

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc04f35
partially fixes xeno examine paper bug
Oct 12, 2023
14cd2d2
...
Oct 13, 2023
ce4b26d
Update code/modules/mob/mob_helpers.dm
Oct 13, 2023
6d66e22
Apply suggestions from code review
Oct 13, 2023
f87fa5d
Merge branch 'master' into xeno-paper
Dec 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions code/modules/mob/mob_helpers.dm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,41 @@ var/global/list/limb_types_by_name = list(
index++
return output_message

// proc that parses an html input string and scrambles the non-html string contents
/proc/stars_decode_html(message)
if(!length(message))
return

// todo: sanitize string more to remove unnencessary <>.
// list of parsed strings that have been scrambled, used to then reinsert the scrambled strings in to the message
var/list/replaced_scrambled_strings = list()

// boolean value to know if the current indexed element needs to be added to the scrambled message.
var/parsing_message = FALSE

// the current string value that needs to be scrambled.
var/current_string_to_scramble = ""

for(var/character_index in 1 to length(message))
if(message[character_index] == ">")
parsing_message = TRUE
continue
if(message[character_index] == "<")
parsing_message = FALSE
Comment on lines +150 to +154
Copy link
Contributor

Choose a reason for hiding this comment

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

You also need to check for quotation marks inside of HTML tags. Consider a tag such as <tag property=">Injection()" />. So when you are inside an HTML tag and run into a quotation mark (either single ' or double "), then you advance until you find the matching quotation mark. Perhaps you could add a couple booleans for this, e.g. need_single_quote and need_double_quote.

Maybe it would be a bit overkill but it's probably good to be safe.

Copy link
Author

Choose a reason for hiding this comment

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

Alright.

if(!length(current_string_to_scramble))
continue
var/scrambled_string = stars(current_string_to_scramble)
replaced_scrambled_strings[current_string_to_scramble] += scrambled_string
current_string_to_scramble = null
continue
if(parsing_message)
current_string_to_scramble += message[character_index]

for(var/unscrambled_message_key in replaced_scrambled_strings)
message = replacetext(message, unscrambled_message_key, replaced_scrambled_strings[unscrambled_message_key])
Copy link
Contributor

Choose a reason for hiding this comment

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

Carefully-crafted user input would make this replace text inside of HTML tags. This should be reworked to be a bit more similar to how /proc/stars works, where the final message is being appended to in the original for loop. In this case, you could be appending scrambled_string from the loop before to a variable representing the final output. I suspect that would also be better performance-wise compared to repeatedly searching through the whole text for replacements.

Your current code also means that duplicates of unscrambled_message_key would get the same star text, while doing it iteratively would preserve the original behavior of not duplicating star text instances.

Copy link
Author

@ghost ghost Dec 28, 2023 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

I'll make it iterative.


return message

/proc/slur(phrase)
phrase = html_decode(phrase)
var/leng=length(phrase)
Expand Down
13 changes: 6 additions & 7 deletions code/modules/paperwork/paper.dm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,23 +78,22 @@
if(in_range(user, src) || istype(user, /mob/dead/observer))
if(!(istype(user, /mob/dead/observer) || istype(user, /mob/living/carbon/human) || isRemoteControlling(user)))
// Show scrambled paper if they aren't a ghost, human, or silicone.
if(photo_list)
for(var/photo in photo_list)
user << browse_rsc(photo_list[photo], photo)
show_browser(user, "<BODY class='paper'>[stars(info)][stamps]</BODY>", name, name, "size=650x700")
onclose(user, name)
read_paper(user, TRUE)
else
read_paper(user)
else
. += SPAN_NOTICE("It is too far away.")

/obj/item/paper/proc/read_paper(mob/user)
/obj/item/paper/proc/read_paper(mob/user, scramble = FALSE)
var/datum/asset/asset_datum = get_asset_datum(/datum/asset/simple/paper)
asset_datum.send(user)
if(photo_list)
for(var/photo in photo_list)
user << browse_rsc(photo_list[photo], photo)
show_browser(user, "<BODY class='paper'>[info][stamps]</BODY>", name, name, "size=650x700")
var/paper_info = info
if(scramble)
paper_info = stars_decode_html(info)
show_browser(user, "<BODY class='paper'>[paper_info][stamps]</BODY>", name, name, "size=650x700")
onclose(user, name)

/obj/item/paper/verb/rename()
Expand Down
Loading