Closed Bug 1483242 Opened Last year Closed Last year

Ensure recording filenames are unique

Categories

(Core :: Web Replay, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bhackett, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
ContentParents use a rather dumb way to pick filenames for temporary recording files and for the recordings saved at exit via the --save-recordings.  The filename is picked based on the number of elapsed milliseconds since the process was created, which can lead to the same filename being picked for multiple different recording processes, with the end result being corrupt recording files.  This is only likely to happen when using the --save-recordings option.

The attached patch fixes this problem by selecting these filenames using a static counter to ensure uniqueness.
Attachment #8999961 - Flags: review?(continuation)
Comment on attachment 8999961 [details] [diff] [review]
patch

Review of attachment 8999961 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentParent.cpp
@@ +777,3 @@
>    nsCOMPtr<nsIFile> file;
>    return !NS_FAILED(NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(file)))
> +      && !NS_FAILED(file->AppendNative(nsPrintfCString("Recording%d", ++sNumTemporaryRecordings)))

You could do "TempRecording%d" here instead to avoid collisions with the non-temp saved recordings, if that might be an issue. On OSX, the temp dir has a weird name nobody will pick, but on Linux I could see somebody picking /tmp/ specifically for non-temp recordings.
Attachment #8999961 - Flags: review?(continuation) → review+
AFAICT, the current method will also have a problem if somebody is recording in multiple instances of Firefox. You could put the PID of the UI process in the file name to work around that.
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/83c0cdea8a9f
Ensure recording filenames are unique, r=mccr8.
https://hg.mozilla.org/mozilla-central/rev/83c0cdea8a9f
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.