Page MenuHomePhorge

Fix error in Mercurial when no offset is specified
Needs RevisionPublic

Authored by jeffrey on May 30 2024, 10:04.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jul 11, 00:14
Unknown Object (File)
Fri, Jul 5, 23:42
Unknown Object (File)
Sun, Jun 30, 15:31
Unknown Object (File)
Sun, Jun 30, 12:02
Unknown Object (File)
Sun, Jun 30, 11:58
Unknown Object (File)
Wed, Jun 26, 05:42
Unknown Object (File)
Fri, Jun 21, 16:15
Unknown Object (File)
Jun 14 2024, 06:03

Details

Summary

When viewing the history of a Mercurial diffusion repository the server return a 500. This is
because a function call to array_slice requires the offset to be set to an integer. So when it is
not specified it can be set to 0 as a default.

The error is as follows:

ERROR 8192: array_slice(): Passing null to parameter #2 ($offset) of type int is deprecated at [/usr/local/www/phorge/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:167]; PHP message: arcanist(head=master, ref.master=3cb117684f4e), phorge(head=master, ref.master=4bf5c452eb28); PHP message:   #0 array_slice(array, NULL) called at [<phorge>/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:167]
Test Plan

The history tab should not return a 500 when not specifying an offset

Diff Detail

Repository
rP Phorge
Branch
master
Lint
Lint Passed
Unit
Tests Skipped
Build Status
Buildable 1307
Build 1307: arc lint + arc unit

Event Timeline

jeffrey requested review of this revision.May 30 2024, 10:04
This revision is now accepted and ready to land.May 30 2024, 10:33
aklapper retitled this revision from Fix error when no offset is specified to Fix error in Mercurial when no offset is specified.May 30 2024, 10:36
aklapper edited the summary of this revision. (Show Details)

Swear I’ve come across this before… it might be fixed in our forked branch so I’ll take a look. Does $limit also need same treatment?

No, limit is working fine as it is.

speck requested changes to this revision.Sat, Jun 22, 03:12

I didn’t find changes in our fork regarding this. We should either change these parameters to be optional in their definition or throw exceptions when they aren’t specified. Changing them to be optional seems the better behavior.

src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php
27

Please change this to optional, along with limit below.

This revision now requires changes to proceed.Sat, Jun 22, 03:12