Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
…анных из TCP-соединения
| else | ||
| { | ||
| retValue = ContextValuesMarshaller.ConvertToCLRObject(val) ?? Missing.Value; | ||
| retValue = ContextValuesMarshaller.ConvertToCLRObject(val); |
There was a problem hiding this comment.
Bug: Undefined values now pass null instead of Missing.Value to COM
The removal of ?? Missing.Value from MarshalIValue changes how Undefined values are passed to COM methods. Previously, Undefined values would be converted to Missing.Value (indicating an optional parameter should use its default), but now they're passed as null. While NotAValidValue correctly returns Missing.Value via ConvertToCLRObject, existing code that passes Undefined to skip optional COM method parameters may now fail or behave differently since null is not equivalent to a missing argument in COM interop.
| Возврат Хост; | ||
| КонецЕсли; | ||
| КонецЦикла; | ||
| КонецФункции No newline at end of file |
There was a problem hiding this comment.
Bug: Missing return value when no hosts available
The function ВыбратьДоступныйХост() has no return statement when none of the candidate hosts are available. If the environment variable is not set (or unavailable) and none of the fallback hosts (localhost, httpbin.org, httpbingo.org) respond, the function exits without returning a value, resulting in Undefined. This causes мАдресРесурса to be set to Undefined, leading to confusing failures in all subsequent HTTP tests that attempt to use the host address.
|
|
||
| if (encoding.Equals(new UTF8Encoding(false))) | ||
| if (encoding.Equals(new UTF8Encoding(false)) || encoding.CodePage == 65001) | ||
| return Utf8NoBOM; |
There was a problem hiding this comment.
Bug: UTF-8 with BOM may be misclassified as without BOM
The CodePage == 65001 fallback was added only to the Utf8NoBOM check (line 96), but not to the Utf8 (with BOM) check on line 93. Both UTF-8 variants share CodePage 65001. If a UTF-8 with BOM encoding has custom fallback settings causing Equals(new UTF8Encoding(true)) to fail, it will pass the CodePage == 65001 check and be incorrectly classified as Utf8NoBOM. The asymmetric application of the CodePage fallback creates potential for encoding misclassification.
| private T ConvertParam<T>(IValue value) | ||
| { | ||
| return ContextValuesMarshaller.ConvertParam<T>(value); | ||
| return ContextValuesMarshaller.ConvertValueStrict<T>(value); |
There was a problem hiding this comment.
Bug: Undefined values now throw when setting typed properties
The change from ConvertParam<T> to ConvertValueStrict<T> introduces a breaking change for Undefined values. The old ConvertParam<T> would return default(T) when given an Undefined value (0 for int, false for bool, null for strings). The new ConvertValueStrict<T> calls ConvertToCLRObject which returns null for Undefined, but then the pattern match converted is T casted fails for null (since null is SomeType is always false in C# pattern matching), causing InvalidArgumentType to be thrown. This affects all context property setters and could break scripts that assign Undefined to typed properties.
Additional Locations (1)
2 participants
Note
Refines COM interop and execution frame/locals handling, adds proper index/type errors, improves TCP read limit, makes HTTP tests auto-select host, and bumps release to 1.9.4.
PrepareReentrantMethodExecution, explicitly set/push frames; new helpersSetFrameLocals/CreateFrameLocalsreturning locals arrays; update local script call path to use them.PrepareDynamicArgsnow accounts forCOMWrapperContextto pass args unchanged.RuntimeException.InvalidIndexType; use it inPropertyNameIndexAccessorfor non-string indices.IVariableargs withVariantWrapperand set out flags; avoid forcingMissing.ValueinMarshalIValue.VariantWrapper).ContextValuesMarshaller.ConvertValueStrict<T>;ConvertToCLRObjectnow returnsMissing.ValueforNotAValidValueand respectsIObjectWrapperfirst.ContextPropertyMappersetters.ReadAllDatanow stops reading when length limit is reached.VariableReferencefields readonly; exposeisIndexed().OS_HTTP_TEST_HOSTor fallbacks) with a pre-test hook; minor comments/notes.ReleaseNumberto1.9.4.Written by Cursor Bugbot for commit 04d9038. This will update automatically on new commits. Configure here.