Skip to content

Commit ae07285

Browse files
committed
fix(shared_poll): validate refresh proxy data against publication_data_format
Data returned by a shared poll refresh proxy reaches subscribers without ever being checked against the channel publication_data_format, so a namespace configured with json/json_object could still deliver payloads clients are not able to parse. Validate the refresh result in the shared poll dispatch and reject the whole batch when any item does not match: the data comes from the application backend, so a mismatch is a developer mistake and must be loud rather than partially applied. The log line names the channel, the offending key and the expected format. Items without data are skipped - a refresh response carries no data both for removals and for keys reported as unchanged, and the dispatch can't tell "unchanged" from "changed to empty" (only the shared poll manager knows the current version of a key). So json/json_object still catch malformed payloads while the empty-data rule of the default format does not apply here.
1 parent 1aa78a3 commit ae07285

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

internal/client/handler.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,14 @@ func (h *Handler) Setup() error {
177177
if !ok {
178178
return centrifuge.SharedPollResult{}, centrifuge.ErrorInternal
179179
}
180-
return handler(ctx, event)
180+
result, err := handler(ctx, event)
181+
if err != nil {
182+
return centrifuge.SharedPollResult{}, err
183+
}
184+
if err := validateSharedPollRefreshData(event.Channel, chOpts.PublicationDataFormat, result.Items); err != nil {
185+
return centrifuge.SharedPollResult{}, err
186+
}
187+
return result, nil
181188
})
182189
}
183190

@@ -979,6 +986,32 @@ func (h *Handler) OnPublish(c Client, e centrifuge.PublishEvent, publishProxyHan
979986
return centrifuge.PublishReply{Result: &result}, err
980987
}
981988

989+
// validateSharedPollRefreshData checks data returned by a shared poll refresh
990+
// proxy against the channel publication_data_format. Data comes from the
991+
// application backend, so a format mismatch is a configuration/implementation
992+
// mistake rather than untrusted input – reject the entire batch loudly instead
993+
// of delivering payloads subscribers are not able to parse.
994+
//
995+
// Items without data are skipped: a refresh response carries no data both for
996+
// removals and for keys the backend reports as unchanged, and this layer can't
997+
// tell "unchanged" from "changed to empty" (only the shared poll manager knows
998+
// the current version of a key). So the empty-data rule of the default format
999+
// is not applied here, while json/json_object still catch malformed payloads.
1000+
func validateSharedPollRefreshData(channel string, format string, items []centrifuge.SharedPollRefreshItem) error {
1001+
for _, item := range items {
1002+
if item.Removed || len(item.Data) == 0 {
1003+
continue
1004+
}
1005+
if err := config.ValidatePublicationData(item.Data, format); err != nil {
1006+
log.Error().Err(err).Str("channel", channel).Str("key", item.Key).
1007+
Str("publication_data_format", format).
1008+
Msg("shared poll refresh data does not match channel publication_data_format, rejecting the whole refresh batch – fix the data returned by the shared poll refresh proxy")
1009+
return centrifuge.ErrorBadRequest
1010+
}
1011+
}
1012+
return nil
1013+
}
1014+
9821015
// OnMapPublish ...
9831016
func (h *Handler) OnMapPublish(c Client, e centrifuge.MapPublishEvent, mapPublishProxyHandler proxy.MapPublishHandlerFunc) (centrifuge.MapPublishReply, error) {
9841017
cfg := h.cfgContainer.Config()

internal/client/handler_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,3 +1826,51 @@ func TestOnMapPublishDataFormat(t *testing.T) {
18261826
})
18271827
}
18281828
}
1829+
1830+
// TestValidateSharedPollRefreshData ensures a shared poll refresh batch is
1831+
// rejected as a whole when the backend returns data not matching the channel
1832+
// publication_data_format, while removals and data-less "unchanged" items pass.
1833+
func TestValidateSharedPollRefreshData(t *testing.T) {
1834+
const ch = "sp:test"
1835+
1836+
t.Run("valid batch accepted", func(t *testing.T) {
1837+
err := validateSharedPollRefreshData(ch, configtypes.PublicationDataFormatJSONObject, []centrifuge.SharedPollRefreshItem{
1838+
{Key: "k1", Data: []byte(`{"v":1}`), Version: 1},
1839+
{Key: "k2", Data: []byte(`{"v":2}`), Version: 1},
1840+
})
1841+
require.NoError(t, err)
1842+
})
1843+
1844+
t.Run("one bad item rejects whole batch", func(t *testing.T) {
1845+
err := validateSharedPollRefreshData(ch, configtypes.PublicationDataFormatJSONObject, []centrifuge.SharedPollRefreshItem{
1846+
{Key: "k1", Data: []byte(`{"v":1}`), Version: 1},
1847+
{Key: "k2", Data: []byte(`[1,2]`), Version: 1},
1848+
{Key: "k3", Data: []byte(`{"v":3}`), Version: 1},
1849+
})
1850+
require.Equal(t, centrifuge.ErrorBadRequest, err)
1851+
})
1852+
1853+
t.Run("invalid json rejected", func(t *testing.T) {
1854+
err := validateSharedPollRefreshData(ch, configtypes.PublicationDataFormatJSON, []centrifuge.SharedPollRefreshItem{
1855+
{Key: "k1", Data: []byte(`not json`), Version: 1},
1856+
})
1857+
require.Equal(t, centrifuge.ErrorBadRequest, err)
1858+
})
1859+
1860+
t.Run("removals and data-less items skipped", func(t *testing.T) {
1861+
// Removals carry no data, and an unchanged key may come back without
1862+
// data — neither must trip the empty-data rule of the default format.
1863+
err := validateSharedPollRefreshData(ch, "", []centrifuge.SharedPollRefreshItem{
1864+
{Key: "k1", Removed: true},
1865+
{Key: "k2", Version: 7},
1866+
})
1867+
require.NoError(t, err)
1868+
})
1869+
1870+
t.Run("binary format allows anything", func(t *testing.T) {
1871+
err := validateSharedPollRefreshData(ch, configtypes.PublicationDataFormatBinary, []centrifuge.SharedPollRefreshItem{
1872+
{Key: "k1", Data: []byte{0x00, 0x01, 0xff}, Version: 1},
1873+
})
1874+
require.NoError(t, err)
1875+
})
1876+
}

0 commit comments

Comments
 (0)