N°9759 - Truncate AttributeText don't work as expected in case of multibytes characters

[accognet]

internal

[greptile-apps]

Greptile Summary

This PR fixes incorrect truncation of multibyte (e.g. emoji) strings in AttributeText fields. Previously, both SetTrim() and the validation check used mb_strlen() (character count) against a TEXT column's byte-based maxSize, so a string of 4-byte characters that fit in far fewer than 65 535 characters could still overflow the column. The fix introduces GetSize() and TrimValue() as polymorphic methods on the attribute definition classes, allowing AttributeString to measure in characters and AttributeText to measure in bytes.

• AttributeDBFieldVoid gets a default GetSize() using mb_strlen(); AttributeText overrides it with strlen() so the size check is now byte-accurate for TEXT columns.
• AttributeString::TrimValue() keeps the existing character-based truncation; AttributeText::TrimValue() uses substr() to find the byte boundary, then steps back one character with mb_substr() to avoid splitting a multibyte sequence.
• webservices/webservices.class.inc.php drops the local TrimAndSetValue() helper in favour of the standard SetTrim(), which is a minor behaviour change: truncated log fields now carry a -truncated (N chars) suffix that the old helper never added.

Confidence Score: 3/5

Safe to merge for all existing callers, but the missing TrimValue() on the base class leaves a gap that will cause a fatal runtime error if SetTrim() is ever called with a non-string attribute.

The core truncation logic in AttributeText::TrimValue() is correct and the tests verify the byte-level boundary behaviour with 4-byte emojis. The delegation in DBObject is clean. However, TrimValue() is not defined on AttributeDBFieldVoid even though GetSize() was placed there — any attribute class that extends the base without going through AttributeString will produce a fatal method-not-found at runtime when SetTrim() is called on it. That gap is real and not guarded anywhere. The variable naming and webservice log behaviour change are minor.

core/attributedef.class.inc.php — the base class AttributeDBFieldVoid needs a TrimValue() implementation or abstract declaration to match the GetSize() that was added there.

Important Files Changed

Filename	Overview
core/attributedef.class.inc.php	Adds GetSize() to AttributeDBFieldVoid (mb_strlen) and TrimValue() to AttributeString (mb_strlen-based) and AttributeText (strlen/byte-based); AttributeText correctly handles multibyte boundaries via a -1 char offset, but TrimValue() is absent from the base class even though GetSize() was added there, and two variables use wrong $s prefix for integer values.
core/dbobject.class.php	Delegates truncation logic from inline code in SetTrim() to $oAttDef->TrimValue(), and replaces hardcoded mb_strlen() with $oAtt->GetSize() in the size validation; clean, correct delegation to the new attribute-level polymorphism.
tests/php-unit-tests/unitary-tests/core/DBObject/DBObjectTest.php	Updates testCheckLongValueInAttribute to accept explicit expected char-lengths and a $bIsValueToSetBelowAttrMaxSize flag instead of computing them inline; test cases for pending_reason (TEXT) now correctly reflect byte-limited truncation with 4-byte emojis.
webservices/webservices.class.inc.php	Removes the local TrimAndSetValue() helper (which truncated silently without any annotation) and replaces all four call sites with SetTrim(); the substitution is correct but adds a -truncated (N chars) suffix to stored log fields that the previous code did not produce.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant DBObject
    participant AttributeDef

    Note over Caller,AttributeDef: SetTrim() delegation (new approach)
    Caller->>DBObject: SetTrim(sAttCode, sValue)
    DBObject->>AttributeDef: MetaModel::GetAttributeDef(class, sAttCode)
    AttributeDef-->>DBObject: oAttDef (AttributeString or AttributeText)
    DBObject->>AttributeDef: oAttDef.TrimValue(sValue)
    alt AttributeString (VARCHAR – char limit)
        AttributeDef->>AttributeDef: "mb_strlen(sValue) > maxSize?"
        AttributeDef-->>DBObject: mb_substr(sValue, 0, maxSize - mb_strlen(msg)) + msg
    else AttributeText (TEXT – byte limit)
        AttributeDef->>AttributeDef: "strlen(sValue) > maxSize?"
        AttributeDef->>AttributeDef: "sVal = substr(sValue, 0, maxSize - strlen(msg))"
        AttributeDef->>AttributeDef: mb_substr(sValue, 0, mb_strlen(sVal) - 1) + msg
        AttributeDef-->>DBObject: "safely truncated value (<=maxSize bytes)"
    end
    DBObject->>DBObject: Set(sAttCode, trimmedValue)

    Note over Caller,AttributeDef: GetSize() delegation (validation)
    Caller->>DBObject: CheckToWrite()
    DBObject->>AttributeDef: oAtt.GetSize(toCheck)
    alt AttributeText
        AttributeDef-->>DBObject: strlen(toCheck) – byte count
    else AttributeString/others
        AttributeDef-->>DBObject: mb_strlen(toCheck) – character count
    end
    DBObject->>DBObject: "iLen > maxSize → String too long"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant DBObject
    participant AttributeDef

    Note over Caller,AttributeDef: SetTrim() delegation (new approach)
    Caller->>DBObject: SetTrim(sAttCode, sValue)
    DBObject->>AttributeDef: MetaModel::GetAttributeDef(class, sAttCode)
    AttributeDef-->>DBObject: oAttDef (AttributeString or AttributeText)
    DBObject->>AttributeDef: oAttDef.TrimValue(sValue)
    alt AttributeString (VARCHAR – char limit)
        AttributeDef->>AttributeDef: "mb_strlen(sValue) > maxSize?"
        AttributeDef-->>DBObject: mb_substr(sValue, 0, maxSize - mb_strlen(msg)) + msg
    else AttributeText (TEXT – byte limit)
        AttributeDef->>AttributeDef: "strlen(sValue) > maxSize?"
        AttributeDef->>AttributeDef: "sVal = substr(sValue, 0, maxSize - strlen(msg))"
        AttributeDef->>AttributeDef: mb_substr(sValue, 0, mb_strlen(sVal) - 1) + msg
        AttributeDef-->>DBObject: "safely truncated value (<=maxSize bytes)"
    end
    DBObject->>DBObject: Set(sAttCode, trimmedValue)

    Note over Caller,AttributeDef: GetSize() delegation (validation)
    Caller->>DBObject: CheckToWrite()
    DBObject->>AttributeDef: oAtt.GetSize(toCheck)
    alt AttributeText
        AttributeDef-->>DBObject: strlen(toCheck) – byte count
    else AttributeString/others
        AttributeDef-->>DBObject: mb_strlen(toCheck) – character count
    end
    DBObject->>DBObject: "iLen > maxSize → String too long"

Loading

_{Reviews (1): Last reviewed commit: "WIP" | Re-trigger Greptile}

[greptile-apps]

Behavior change: truncated webservice log fields now include a trailing message

The removed TrimAndSetValue() silently cut the value with mb_substr($sValue, 0, $iMaxSize) and stored it without any annotation. The new SetTrim() path appends " -truncated (N chars)" to the stored value. Any log parser, monitoring tool, or test that reads log_info, log_warning, log_error, or data on WebServicesLog objects and relies on the raw truncated content would now see this extra suffix. If the behavior change is intentional, a comment or changelog note would help future maintainers understand the difference.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

TrimValue() is missing from the base class that exposes GetSize()

GetSize() is added to AttributeDBFieldVoid, but TrimValue() is only defined on AttributeString (and overridden in AttributeText). DBObject::SetTrim() now calls $oAttDef->TrimValue($sValue) on any attribute returned by MetaModel::GetAttributeDef(). All current callers happen to use string/text attributes, but any future call to SetTrim() on an attribute class that extends AttributeDBFieldVoid without going through AttributeString (e.g. a custom attribute type) will trigger a fatal Call to undefined method error at runtime. Adding a default TrimValue() implementation to AttributeDBFieldVoid — or at least an abstract declaration — would make the interface consistent with GetSize() and prevent the silent runtime failure.

[Copilot]

Pull request overview

This PR fixes issue N°9759, where truncation of AttributeText fields did not behave correctly for multibyte characters. Previously, text field size limits were checked and truncated using mb_strlen/mb_substr (character counts), but MySQL TEXT columns are limited by bytes, not characters. As a result, values containing multibyte characters (e.g. emojis) could exceed the column's byte capacity and be silently rejected/corrupted. The fix introduces per-attribute-type size semantics: character-based for AttributeString (VARCHAR) and byte-based for AttributeText/AttributeLongText (TEXT).

Changes:

• Added GetSize() and TrimValue() methods on attribute definitions: char-based on AttributeString/AttributeDBFieldVoid, byte-based (with multibyte-safe truncation) on AttributeText.
• Refactored DBObject::SetTrim() and CheckValue() to delegate size/trim logic to the attribute definition, and removed the now-redundant TrimAndSetValue() helper in webservices.
• Updated the unit test data provider and signature to assert byte-aware expected truncation lengths.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
core/attributedef.class.inc.php	Adds GetSize/TrimValue with char-based (String) and byte-based (Text) truncation logic
core/dbobject.class.php	SetTrim and CheckValue now delegate to the attribute's TrimValue/GetSize
webservices/webservices.class.inc.php	Removes local TrimAndSetValue helper, reuses SetTrim
tests/php-unit-tests/unitary-tests/core/DBObject/DBObjectTest.php	Extends provider with explicit expected-length/below-max flags for multibyte cases

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.