SLD: Improve WMS GetStyles request #5832
Conversation
|
I don't have any objections here. This is really limited to WMS/SLD and doesn't result in anything that we might want to expose more broadly (unlike the other 2 pull requests). Could anything sensitive be divulged through the XML generation? I can't think of anything beyond attribute bindings or expressions already defined in the mapfile. I mean, what if style SIZE is already referencing an expression (based on the other pull request). |
mapogcsld.c
Outdated
| psLabelText = NULL; | ||
| if (psLabelObj->text.string) | ||
| { | ||
| psLabelText = msStrdup(psLabelObj->text.string); |
There was a problem hiding this comment.
what if psLabelObj->text.string doesn't start with [ and ends with ] ? Should the type of the expression be checked ?
There was a problem hiding this comment.
You are correct. In my attempt to upgrade this part of code to MapServer version 6.2, I missed the whole picture and handled only few simple use cases. I'll rework on this as soon as I can. Changes may involve an evaluation of the expression. Up to you to decide whether the merge needs these upcoming changes or if it can be performed as is and changes be applied in another pull request.
mapogcsld.c
Outdated
| } | ||
| else if (psClass->text.string) | ||
| { | ||
| psLabelText = msStrdup(psClass->text.string); |
mapogcsld.c
Outdated
| } | ||
| if (!psLabelText) continue; // Can't find text content for this <Label> | ||
|
|
||
| psLabelText = msReplaceSubstring(psLabelText, "\"", ""); |
There was a problem hiding this comment.
what is this replacement for ?
mapogcsld.c
Outdated
| } | ||
| else if (psLayer->labelitem) | ||
| { | ||
| psLabelText = msStrdup(psLayer->labelitem); |
There was a problem hiding this comment.
shouldn't that be XML escaped ?
|
@jbo-ads what is the status of this change? It seems you mentioned that you will 'rework' this. |
|
Actually I am currently on it. I already solved merge conflicts on my local repo. Moreover, comments from @rouault are trickier than they look like... I need to update |
|
@jbo-ads ok thanks for the update. Do you want me to wait for your changes, before beginning the 7.6.0 release process? |
|
meaning: ok I will wait for your changes, before proceeding (unless you advise otherwise here). |
|
Thank you, but I can wait for mid-year's 8.0 release for my changes to be integrated. I think that many people are waiting for a 7.6. |
jbo-ads
force-pushed
the
sld-getstyles
branch
from
21ff2ef
to
b71694e
Compare
|
@rouault : I updated the code to handle labels of both types MS_STRING and MS_EXPRESSION. I didn't include handling of Mapfile functions though. I don't know what to do with them:
|
|
@jbo-ads I let you issue a backport PR against branch 7.6 if you wish |
This pull request aims at improving WMS GetStyles request, especially when a SLD or SLD_BODY parameter is present. It implements the second section of MS RFC 124: Improving SLD Support in MapServer.