View Issue Details

IDProjectCategoryView StatusLast Update
0015870mantisbtapi soappublic2017-09-03 05:31
Reportervboctor Assigned Tovboctor  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionwon't fix 
Product Version1.2.15 
Summary0015870: 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:

  • Fix the serialization / deserialization issues.
  • Come up for an easy way for SOAP clients to get a value that is easy to process without much hassle around parsing.
TagsNo tags attached.

Relationships

related to 0015835 acknowledged MantisTouch handle_bug_threshold is a serialized array - issue_edit_page.php not working 

Activities

dregad

dregad

2013-05-17 10:11

developer   ~0036891

  • Fix the serialization / deserialization issues.

This should be as easy as


diff --git a/api/soap/mc_config_api.php b/api/soap/mc_config_api.php
index c4c1ab6..40137cf 100644
--- a/api/soap/mc_config_api.php
+++ b/api/soap/mc_config_api.php
@@ -24,6 +24,6 @@ function mc_config_get_string( $p_username, $p_password, $p_config_var ) {
return SoapObjectsFactory::newSoapFault( 'Client', "Config '$p_config_var' is undefined" );
}

  • return config_get( $p_config_var );
  • return json_encode( config_get( $p_config_var ) );
    }

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.

Come up for an easy way for SOAP clients to get a value that is easy to process without much hassle around parsing.

IMO, json is quite standard and should be easy enough to parse. Do you think something else is needed ?

rombert

rombert

2013-05-17 15:18

reporter   ~0036892

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.

dregad

dregad

2013-05-17 17:42

developer   ~0036894

First of all, I don't think it's conceptually correct to return JSON in a SOAP message.

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.

Second of all, we're going to break all existing clients which rely on this call.

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.

vboctor

vboctor

2013-05-18 02:25

manager   ~0036895

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:

  1. +1 to @rombert's suggestion of returning a soap complex structure that clients can easily have proxies for to know the type and deal with it without parsing. Once we add this new method, we can mark the older one as deprecated, though leave it in for backward compatibility.

  2. Reduce the cases where a client needs to enforce business rules. For example, the scenario that is broken here is that the client gets a threshold from MantisBT then uses that to filter user list to get list of users that can be assigned an issue. Then use these to populate the handler combo box in the edit form. In my opinion, the client should just ask the question: project_get_users( 'handler', $p_project_id ) and get back an array of accounts.

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#.

rombert

rombert

2013-05-18 09:19

reporter   ~0036896

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.

vboctor

vboctor

2014-02-16 20:24

manager   ~0039425

Here is a pull request with a proposed simple fix:
https://github.com/mantisbt/mantisbt/pull/141

We can do more elaborate fixes for adding new apis that return structured data in addition to the fix in this pull request.

vboctor

vboctor

2014-10-26 02:54

manager   ~0041667

Here is another pull request that goes beyond the scope of the original one and uses json serialization:
https://github.com/mantisbt/mantisbt/pull/528

florant

florant

2017-03-08 07:50

reporter   ~0055988

Hi,
I'm currently using the SOAP API and I would like to obtain the config $g_bug_view_page_fields that is an Array.
Is it planned to develop this feature ?

rombert

rombert

2017-08-17 16:13

reporter   ~0057488

@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"?

vboctor

vboctor

2017-08-17 17:51

manager   ~0057490

Resolving as won't fix, since the REST API already has much better handling for retrieving of configuration.