Closed Bug 1586070 Opened 4 months ago Closed 3 months ago

Read does not advance file position for files larger than 4GB on Win32.

Categories

(NSPR :: NSPR, defect, P2)

4.19
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbordas, Assigned: KaiE)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Attached file LargeFileRead.zip

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36

Steps to reproduce:

Once trying to read a large file beyond the 0xFFFFFFFF position using PR_READ the file position is not advanced:

// Read test.
PROffset64 offset, position;

PRFileDesc *fd = PR_Open(largeFilePath.c_str(), PR_RDONLY, 0777);

if (fd == NULL) {
    return -1;
}

offset = 0xFFFF + 16;

PR_Seek64(fd, offset, PR_SEEK_SET);

PR_Read(fd, tempData.data(), 1);
offset++;

PR_Read(fd, tempData.data(), 1);
checkValue(offset, tempData[0]);
// No problem, position is below 4GB.
offset++;

offset = (PROffset64)0xFFFFFFFF + 16;

PR_Seek64(fd, offset, PR_SEEK_SET);

PR_Read(fd, tempData.data(), 1);
offset++;

PR_Read(fd, tempData.data(), 1);
// Returns the same buffer as the previous PR_Read.
checkValue(offset, tempData[0]);

PR_Close(fd);

Problem in ntio.c:

At line 2289:

        SetFilePointer((HANDLE)f, me->md.blocked_io_bytes, 0, FILE_CURRENT);

... needs to be replaced with the same code applied to line 2442:

        /*
         * Moving the file pointer by a relative offset (FILE_CURRENT)
         * does not work with a file on a network drive exported by a
         * Win2K system.  We still don't know why.  A workaround is to
         * move the file pointer by an absolute offset (FILE_BEGIN).
         * (Bugzilla bug 70765)
         */
        offset.LowPart = me->md.overlapped.overlapped.Offset;
        offset.HighPart = me->md.overlapped.overlapped.OffsetHigh;
        offset.QuadPart += me->md.blocked_io_bytes;

        SetFilePointer((HANDLE)f, offset.LowPart, &offset.HighPart, FILE_BEGIN);

Actual results:

The PR_Read will return the same buffer over and over instead of advancing the file position.

Expected results:

PR_Read should advance the file position and return the associated bytes just like it does for positions below 4GB.

The priority flag is not set for this bug.
:jcj, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jjones)

Thanks for reporting this. Could I ask, what led to this discovery?

Flags: needinfo?(jjones) → needinfo?(zbordas)
Priority: -- → P2

(In reply to J.C. Jones [:jcj] (he/him) from comment #2)

Thanks for reporting this. Could I ask, what led to this discovery?

We are running a C++ based peer-to-peer application dealing with large video files and we discovered this issue.

Flags: needinfo?(zbordas)

Thanks for your report.
Applying the existing fix in _PR_MD_WRITE from bug 70765 also to _PR_MD_READ seems reasonable.

Assignee: nobody → kaie
Attached patch 1586070-v1.patch (obsolete) — Splinter Review

zbordas, can you confirm that my attachment is the change that you're suggesting?

Flags: needinfo?(zbordas)
Attachment #9105377 - Attachment is obsolete: true
See Also: → 70765

(In reply to Kai Engert (:kaie:) from comment #6)

zbordas, can you confirm that my attachment is the change that you're suggesting?

Yes, this is the exact change I made and tested in my local environment!

Flags: needinfo?(zbordas)

I'm trying to get bug 1579836 fixed prior to landing this one.

See Also: → 1579836
Comment on attachment 9105379 [details] [diff] [review]
1586070-v2.patch [contributed by zbordas]

This patch was technically provided by zbordas - so I can give it r+.
Attachment #9105379 - Attachment description: 1586070-v2.patch → 1586070-v2.patch [contributed by zbordas]
Attachment #9105379 - Flags: review+
Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → 4.24
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: 4.24 → 4.25
Blocks: 1600803
You need to log in before you can comment on or make changes to this bug.