The engine accepts a booking line that names no tariff even on a resource that sells them, and such a line is neither priced by a tariff nor capped by one.

The guard in BookingManager that rejects a tariff belonging to another resource is written as a conjunction beginning $tariff !== NULL, so it only fires when a tariff was given. A line with no tariff passes it untouched. Two things follow:

  • It is priced from the resource. PaymentPolicyResolver::policyFor() walks the tariff and then the resource and takes the first value set, and with no tariff there is nothing to prefer.
  • Its only ceiling is the slot's own capacity. assertCapacity() uses the slot cap when no tariff is named, and assertLimit() returns immediately when the capacity is NULL. So on a slot with no overall capacity the line is unbounded.

NULL is the normal state for exactly the resources at risk: the slot generator gives the overall capacity only to resources that define no tariffs, while resources that do define them get a slot tariff each and no slot-wide cap. A tariffed resource therefore typically has slots where an untariffed line is both unpriced by any tariff and uncapped.

How reachable is it

Stated carefully, because this decides the severity rather than the correctness.

SlotBookingForm::desiredQuantities() iterates every submitted quantity key and maps the key slot to the tariff-less target token, with no check that the resource actually sells no tariffs. What stops a crafted submission today is that Drupal's form API only populates values for elements it built, so a quantity[slot] key on a tariffed slot should be dropped before that method sees it.

That protection is incidental rather than intended, it sits in the wrong layer, and it has not been proven. It would also disappear the day a headless entry point or a different rebuild repopulates values another way.

There is no HTTP surface making it worse: yoyaku_api declares no routes and no REST resource, so it is a PHP facade for trusted callers, and the admin place-booking form is permission-gated.

Proposed fix: let a resource declare that it sells tariffs

Guarding on whether the slot has slot tariffs would make the engine answer "does this sell tariffs?" by counting rows, and answer it per slot. Both readings get the same case wrong: a resource that intends to sell tariffs but has none yet, or whose slots have not been generated, is indistinguishable from one that never will.

So add a tariffs_enabled boolean to yoyaku_resource, named after the resource's existing guarantee_enabled rather than inventing a second pattern. It is per resource only, deliberately not resolved through the resource type and a site default the way seating_strategy is: that is a lot of machinery for one bit, and a resource type covering both kinds is perfectly normal.

The guard then reads the declaration instead of counting: a line naming no tariff is refused when its resource has tariffs enabled. That is unambiguous, cheap, correct before any slot exists, and it holds for every surface at once rather than hardening one form and hoping the next caller remembers.

It also lets the interface stop offering things that do nothing: the Tariffs tab appears only when tariffs are enabled, and the resource price field is disabled when they are, so a price lives in exactly one place.

The states the flag makes possible

  • Enabled with no tariffs yet. Legitimate while setting a resource up, so not a validation error. The resource sells nothing until a tariff exists, and the Tariffs tab should say that plainly rather than showing an empty table.
  • Disabling it while tariffs exist. Refuse, with a validation error naming how many there are. Silently orphaning the rows would leave data nothing reads, and silently deleting them would throw away an operator's pricing.
  • Existing sites. Derive the flag once, enabled where the resource already has at least one tariff. That is the single moment the row-counting inference is right, because it describes what the site is doing today.

What does not change

The slot capacity keeps bounding the total across tariffs: assertCapacity() enforces it in addition to the tariff's own cap, so a capacity is still meaningful with tariffs enabled. The price is either/or; the capacity is both.

Testing

Write the test first and see it fail against unfixed code, including a posted quantity[slot] on a resource with tariffs enabled. That settles the form API question with evidence instead of leaving it as an assumption.

Notes

Reported as a normal public issue rather than through the security process: the project's releases are alpha, which sits outside the security advisory policy.

Found while designing #3614386: Price a production from a reusable tariff preset instead of retyping its tariffs. This also turns #3615067: A resource tariff must carry its own price, so the resource price cannot silently apply from a safety improvement into a coherent rule, since with the flag it is a declaration rather than an empty field that decides which of the two prices applies, so #3615067: A resource tariff must carry its own price, so the resource price cannot silently apply should depend on this issue.

Issue fork yoyaku-3615068

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Issue summary: View changes

Summary updated: the fix now has the resource declare that it sells tariffs, instead of the engine inferring it.

The original proposal refused a tariff-less line when the slot offered slot tariffs. That makes the engine answer "does this sell tariffs?" by counting rows, and answer it per slot, and both readings get the same case wrong: a resource that intends to sell tariffs but has none yet, or whose slots have not been generated, is indistinguishable from one that never will. So the guard would have been correct only once a site was fully set up.

Instead, a tariffs_enabled boolean on yoyaku_resource, named after the existing guarantee_enabled. Per resource only, deliberately not resolved through the resource type and a site default the way seating_strategy is: that is a lot of machinery for one bit, and a resource type covering both kinds is perfectly normal.

Two things fall out of it beyond the guard. The Tariffs tab can appear only when tariffs are enabled, and the resource price field can be disabled when they are, so a price lives in exactly one place. The summary also records what happens in the states a flag makes possible: enabled with no tariffs yet is legitimate mid-setup, disabling it while tariffs exist is refused rather than orphaning or deleting them, and an existing site derives the flag once from whether a tariff already exists.

Checked before adding a field rather than after: booking_mode is a different axis, how a requested quantity becomes bookings, and nothing else on the resource carries this. Also worth being explicit that the slot capacity is unaffected, since assertCapacity() enforces it in addition to a tariff's own cap. The price is either/or; the capacity is both.

#3615067: A resource tariff must carry its own price, so the resource price cannot silently apply should depend on this: with the flag, which of the two prices applies is settled by a declaration rather than by which field happens to be empty.

  • mably committed 2a43c247 on 1.x
    fix: #3615068 A booking that names no tariff is accepted on a resource...
mably’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.