Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing fields #27

Closed
jixam opened this issue Nov 5, 2022 · 5 comments
Closed

Missing fields #27

jixam opened this issue Nov 5, 2022 · 5 comments

Comments

@jixam
Copy link

jixam commented Nov 5, 2022

Since v0.70 we are missing some RT::Client::REST::Ticket fields.

Our RT response looks like this, using cat -E to show line endings:

curl 'https://rt/REST/1.0/ticket/971216' | cat -E

RT/4.4.4 200 Ok$
$
id: ticket/971216$
Queue: whatever$
Owner: Nobody$
Creator: someone@example.com$
Subject: Problems$
Status: new$
Priority: 10$
InitialPriority: 10$
FinalPriority: 50$
Requestors: someone@example.com$
Cc:$
AdminCc:$
Created: Fri Nov 04 15:38:18 2022$
Starts: Not set$
Started: Not set$
Due: Sun Nov 06 15:38:18 2022$
Resolved: Not set$
Told: Not set$
LastUpdated: Fri Nov 04 16:19:43 2022$
TimeEstimated: 0$
TimeWorked: 0$
TimeLeft: 0$
CF.{AdminURI}: $
$

It turns out that the empty Cc: field is treated as invalid and aborts the reading of the following fields.

Our problem is fixed by this small patch:

--- a/lib/RT/Client/REST/Forms.pm
+++ b/lib/RT/Client/REST/Forms.pm
@@ -71,7 +71,7 @@
                 next LINE
             }

-            if ($state <= 1 && $line =~ m/^($field: )(.*)?$/s) {
+            if ($state <= 1 && $line =~ m/^($field: ?)(.*)?$/s) {
                 # Read a field: value specification.
                 my $f     = $1;
                 my $value = $2;
@AnneBennett
Copy link

FWIW, I had exactly the same problem, and created exactly the same patch independently. I should have searched here first! :-)

@djzort
Copy link
Contributor

djzort commented Nov 19, 2022

The regex above would accept output like this:

Field:data

Which as far as i can tell is incorrect. There is no spec so unfortunately it is just whatever RT is doing.

Try this fix instead which treats empty fields as their own case

feca968

@djzort
Copy link
Contributor

djzort commented Dec 5, 2022

0.71 released to the cpan

@djzort djzort closed this as completed Dec 5, 2022
@jixam
Copy link
Author

jixam commented Feb 13, 2023

Unfortunately, that change seems to be incomplete. Upgrading to 0.71, we are now getting two new warnings:

Use of uninitialized value $value in pattern match (m//) at RT/Client/REST/Object.pm line 285.
Use of uninitialized value $value in split at RT/Client/REST/Object.pm line 307.

We think this patch on 0.71 would fix things but it sounds like you do not want to go that way. In that case I guess all calls will have to be reviewed to properly handle undef? (I didn't check to see if there are more cases than the two we are currently experiencing)

--- /usr/local/lib/perl5/site_perl/5.36.0/RT/Client/REST/Forms.pm
+++ Forms.pm
@@ -77,7 +77,7 @@
                 $f =~ s/:?$//;

                 push(@$o, $f) unless exists $k->{$f};
-                vpush($k, $f, undef);
+                vpush($k, $f, "");

                 $state = 1;

@djzort
Copy link
Contributor

djzort commented Feb 27, 2023

0.72 should hopefully resolve this. I would rather empty fields be rendered as undef instead of q() - which follows this precedent https://github.com/RT-Client-REST/RT-Client-REST/blame/bc93ab3854202eb863fb4bd5a42297a67234b2d0/lib/RT/Client/REST/Object.pm#L496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants