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

Add sound flag checking to soundExists #3133

Merged
merged 4 commits into from
Sep 19, 2024
Merged

Add sound flag checking to soundExists #3133

merged 4 commits into from
Sep 19, 2024

Conversation

thegrb93
Copy link
Contributor

@thegrb93 thegrb93 commented Sep 6, 2024

No description provided.

flags = nil
end

path = string.GetNormalizedFilepath(path)
if istable(sound.GetProperties(path)) or file.Exists("sound/" .. path, "GAME") then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to make sure sound.GetProperties doesn't accept the unallowed flags

@@ -1537,9 +1537,20 @@ if not WireLib.PatchedDuplicator then
end

function WireLib.SoundExists(path)
path = string.GetNormalizedFilepath(string.gsub(string.sub(path, 1, 260), '["?]', ''))
-- Limit length and remove invalid chars
if #path>260 then path = string.sub(path, 1, 260) end
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for having this? iirc this was only put into place as a band aid fix for sounds too long for source to play, but since now there's file path checking isn't it unnecessary?

@unknao
Copy link
Contributor

unknao commented Sep 7, 2024

It seems like there were people who made use of sound emitters & e2 playing sounds that aren't on the server, maybe having a convar to disable the file checking would be good enough to please such people.

@thegrb93
Copy link
Contributor Author

thegrb93 commented Sep 9, 2024

I need to test if there's memory leaking issue with invalid paths.

@wrefgtzweve
Copy link
Contributor

wrefgtzweve commented Sep 14, 2024

there is, that was the point of checking if the file exists.
i'll dm you a example e2 on discord to do the abuse

@thegrb93
Copy link
Contributor Author

can probably make a per player limit of sound paths instead. not ideal but probably better

@thegrb93
Copy link
Contributor Author

@unknao Try this

@thegrb93 thegrb93 merged commit ee7e76e into master Sep 19, 2024
1 check failed
@thegrb93 thegrb93 deleted the sound-exists branch September 19, 2024 08:26
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.

3 participants