Skip to content

Pagination stops silently when a rel=next Link is present but unparseable, returning truncated data as success #356

Description

@rolandbanks

Summary

The pagination loop in MerakiClientPager.cs treats "I could not parse the rel=next link" identically to "there is no next page". When a rel=next link is present but does not split into exactly two parts on ;, the loop stops, returns the pages fetched so far, and reports success. No exception is thrown and nothing is logged.

The caller receives a clean HTTP 200 result that is silently short.

Location

Meraki.Api/MerakiClientPager.cs, in all six pagination methods. Using the two-cursor ApiResponse overload (v1.70.37, lines 164-195):

if (pageResponse.Headers is not null && pageResponse.Headers.TryGetValues("Link", out var linkHeaders))
{
    var linkHeader = linkHeaders.FirstOrDefault();
    if (linkHeader != null)
    {
        var links = linkHeader.Split(',');
        var nextLink = links.SingleOrDefault(link => link.Contains("rel=next"));
        if (nextLink != null)
        {
            var nextLinkComponents = nextLink.Split(';');
            if (nextLinkComponents.Length == 2)          // <-- line 175
            {
                ...
                continue;                                 // the ONLY way to fetch another page
            }
        }
    }
}

// There was no Link header so we're finished
finished = true;                                          // <-- line 195, reached by fall-through

The loop only continues via the continue, nested four ifs deep. Failing any of the four falls through to finished = true.

Three of those four are legitimate stop conditions: no Link header, or a Link header with no rel=next, is how the API signals the last page.

The fourth is not. At nextLinkComponents.Length == 2 the API has explicitly told us there is another page and the library stops anyway. The comment at line 194 ("There was no Link header so we're finished") is accurate for the other three routes but misleading for this one.

Ways a rel=next segment could fail to split into exactly two parts:

  • an additional attribute, for example <url>; rel=next; title="..." gives 3
  • a ; inside the URL or its query string gives 3 or more

Why this matters

This is a silent data-loss path. A consumer that reconciles a local cache against the returned collection, removing anything not present upstream, would discard precisely the items on the pages that were never fetched, while every signal available to it says the call succeeded.

Crucially, no defensive check on the calling side can detect this. The returned list is non-empty, so a zero-count guard does not fire, and "fewer items than last time" is also what a legitimate deletion looks like. From the caller's perspective a truncated response is indistinguishable from a correct one.

Related, minor

Line 171 uses links.SingleOrDefault(link => link.Contains("rel=next")), which throws InvalidOperationException if more than one segment matches. That is not an ApiException, so consumers filtering on ApiException/HttpStatusCode will not catch it. It fails safe (the operation aborts rather than returning bad data), so it is much less serious than the truncation above, but FirstOrDefault would be the more forgiving choice.

Suggested fix

Distinguish "no next page" from "could not parse the next page link":

  • If no rel=next segment is present, finish as now. This is the normal terminating case.
  • If a rel=next segment is present but cannot be parsed, throw rather than returning silently truncated data. Failing loudly is strictly safer than under-reporting, since the caller can retry.

Parsing the segment more tolerantly would also help: take the first ;-delimited component as the URL and scan the remainder for the rel attribute, rather than requiring exactly two components.

Version

Observed in 1.70.37. The relevant code is unchanged on main as of this writing.

🤖 Filed with the assistance of Claude Code

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions