Skip to content

Commit

Permalink
[BUGFIX] 13118 QGIS Server - WFS - GeoJSON and escaping line breaks
Browse files Browse the repository at this point in the history 
Line breaks are not properly handled in GeoJSON results when making GetFeature requests.
Line breaks should be replaced by \\n.
  • Loading branch information
@rldhont
rldhont committed Nov 11, 2015
1 parent b4d206a commit 016f497
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/server/qgswfsserver.cpp
View file
Expand Up @@ -1751,7 +1751,10 @@ QString QgsWFSServer::createFeatureGeoJSON( QgsFeature* feat, int prec, QgsCoord
else
{
fStr += "\"";
fStr += val.toString().replace( QString( "\"" ), QString( "\\\"" ) );
fStr += val.toString()
.replace( QString( "\"" ), QString( "\\\"" ) )

This comment has been minimized.

Sign in to view

Copy link
@nyalldawson

nyalldawson Nov 11, 2015

Collaborator

Please use replace( '\n',... ) instead. The single quote character methods are MUCH faster than the Qstring versions.

This comment has been minimized.

Sign in to view

Copy link
@rldhont

rldhont Nov 12, 2015

Author Contributor

hi @nyalldawson, Thanks! I don't know about this.
Does it also work for \r and " ?

This comment has been minimized.

Sign in to view

Copy link
@nyalldawson

nyalldawson Nov 12, 2015

Collaborator

Yep, any single character (or single escaped character) will work. They take about 1/10th of the time of the string variant.
.replace( QString( "\r" ), QString( "\\r" ) )
.replace( QString( "\n" ), QString( "\\n" ) );
fStr += "\"";
}
fStr += "\n";
Expand Down

6 comments on commit 016f497

@jef-n
Copy link
Member

@jef-n jef-n commented on 016f497 Nov 12, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also run "scripts/prepare-commit.sh" (it can also directly be linked to .git/hooks/pre-commit) - and include "fix #ticket" or "fixes #ticket" in the commit message so that redmine picks it up automatically.

@rldhont
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jef-n I thought there is a conflict between redmine and github about "fix #ticket".
I think this string will close a github issue or pull-request and the good redmine issue. am I wrong ?

@jef-n
Copy link
Member

@jef-n jef-n commented on 016f497 Nov 12, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. fix #xxx closes the redmine issue. I doubt there is a (enabled) automation for closing github issues (which we currently don't use for qgis anyway) or PRs. But the github web interface still mistakes #xxx in commits as links to PRs - but that's harmless.

@rldhont
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use 'fixes #xxxx' to close github issues in my other projects, but there is also fix, fixed, closes, close, closed, resolve, resolves, resolved https://help.github.com/articles/closing-issues-via-commit-messages/ . I don't know if we can deactivate its

@jef-n
Copy link
Member

@jef-n jef-n commented on 016f497 Nov 12, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we use "fixes #xxxx" to close redmine issues (I think also Fix,implements - would have to look it up in redmine). See all over the commit log. I didn't hear from anyone that something else was closed by those - so I suppose it is not activated.

@rldhont
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll do it.

Please sign in to comment.