View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0015870 | mantisbt | api soap | public | 2013-05-17 00:31 | 2017-09-03 05:31 |
Reporter | vboctor | Assigned To | vboctor | ||
Priority | normal | Severity | minor | Reproducibility | have not tried |
Status | closed | Resolution | won't fix | ||
Product Version | 1.2.15 | ||||
Summary | 0015870: Deserialization issues caused by mc_config_get_string() | ||||
Description | This issue was reported in 0015835 Some configuration options like handle_bug_threshold can be set to an integer (e.g. 55 for developer) or array (e.g. array(55, 70)). When it is set to an array, it is serialized fine, but the client fails deserializing the array as a string. Two issues to resolve here:
| ||||
Tags | No tags attached. | ||||
related to | 0015835 | acknowledged | MantisTouch | handle_bug_threshold is a serialized array - issue_edit_page.php not working |
This should be as easy as
Of course this relies on json extension to be installed on the MantisBT soap host (which should be the case by default for PHP >= 5.2.0). For belts and braces, we could check for existence of json_encode function, and trigger an error if it does not exist.
IMO, json is quite standard and should be easy enough to parse. Do you think something else is needed ? |
|
Well, we can't do that over SOAP. First of all, I don't think it's conceptually correct to return JSON in a SOAP message. Second of all, we're going to break all existing clients which rely on this call. I think the right way to do this is to create a new SOAP method ( mc_config_get perhaps ) which returns a complex data structure. |
|
That was just a suggestion. I'm by no means a SOAP expert, so if you say it's not correct to use JSON then I'll rest my case.
Well, considering that today if the config is an array the function call throws an exception (as the reported bug shows), I'd say it's already broken and pretty much useless as it is since it can only successfully return integer and string types. Keep in mind that the client has no way of knowing in advance what data type is actually contained in the config it's querying for. I'm looking forward to learning how you would properly address this issue. |
|
At the moment we are broken. However, I would say 98% of the cases it is not an array. So we don't want to replace 2% broken with 100% broken :) I suggest the following:
For this specific scenario, I prefer the 2nd approach. However, we may find other scenarios where it makes sense to implement the 1st approach as well. The goal is to have clients as dumb as possible and keep the intelligence on the MantisBT, hence, have consistency of behavior and less round trips between the client and MantisBT to attempt to have a consistent behavior. The reporter of the issue suggested using xsd:anytype rather than xsd:string. This may work for PHP (possibly - not sure), but I see it as a problem for languages that generate a proper proxy like Java and C#. |
|
If we can narrow down the types of data we send using mc_config_get_string we can think of a serialization scheme which does not break existing clients. We can serialize arrays with a separator we know will not be used. As an example, we can serialize '50' as '50' and 'array(50,70)' as '50|70'. But this assumes that either '|' is never part of a config string or that we escape it. And it also assumes that we only use arrays whose keys are not meaningful. Reading back what I wrote, I still think we should have a SOAP-based serialisation. As for the xsd:any change, this is a breaking change for Java/C# and possibly others, like Victor mentioned. And I fully agree that we should aim for methods which remove the need for clients to get configuration values. |
|
Here is a pull request with a proposed simple fix: We can do more elaborate fixes for adding new apis that return structured data in addition to the fix in this pull request. |
|
Here is another pull request that goes beyond the scope of the original one and uses json serialization: |
|
Hi, |
|
@florant - I expect that new features will be added to the REST API rather than to the SOAP API. @vboctor - would it make sense to resolve this as a "Won't fix"? |
|
Resolving as won't fix, since the REST API already has much better handling for retrieving of configuration. |
|