Skip Menu |

This queue is for tickets about the Business-OnlinePayment-PayflowPro CPAN distribution.

Report information
The Basics
Id: 48816
Status: resolved
Priority: 0/
Queue: Business-OnlinePayment-PayflowPro

People
Owner: PLOBBES [...] cpan.org
Requestors: josh [...] infogears.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Request Encoding Issue & Missing fields
Date: Tue, 18 Aug 2009 15:59:06 -0600
To: bug-Business-OnlinePayment-PayflowPro [...] rt.cpan.org
From: Josh Rosenbaum <josh [...] infogears.com>
The attached patch is based on my last patched file. (This is a problem with BOP::PayflowPro 0.07.) Bug Details: Since we were using the HTTPS module, it was URL encoding our submission. This is not allowed by Paypal, however, and was causing problems, because they do not URL decode the submission. So the attached patch will use their bracketed numbers scheme and directly create the request text itself for the HTTPS module to use. Patch details: *) Patched encoding so we work correctly. *) Added missing shipping parameters for content. (Added to documentation as well.) (I used the same mapped fields that the Authorize.Net module uses so we match up.) *) customer_id was mentioned in the documentation, but there was no mapping so it didn't do anything. I have mapped this to CUSTCODE now. *) This next one is controversial and I almost took it out myself as I'm not sure it's a perfect idea, since we sort of want the user to follow the BOP API. I'll leave the decision up to you. --- We no longer overwrite values when doing mapping. For example, if a user manually enters COMMENT2, but does not enter the invoice_number, we previously would wipe their COMMENT2 value and not use it. With this patch, we recognize they didn't enter invoice_number and we don't overwrite the COMMENT2 value the user specified. This is the first part of the patch if you want to remove: @@ -139,6 +139,14 @@ -- Josh
--- PayflowPro.pm.orig_patched 2009-08-18 15:24:33.000000000 -0600 +++ PayflowPro.pm.new_encoding 2009-08-18 15:32:37.000000000 -0600 @@ -139,6 +139,14 @@ my ( $self, %map ) = @_; my %content = $self->content(); foreach ( keys %map ) { + if ( !ref( $map{$_} ) && !defined( $content{ $map{$_} } ) ) { + + # If there is nothing in the content to map, let's not overwrite any + # possible existing values. Example: if the user doesn't specify + # invoice_number, but does specify COMMENT2, we should use the + # COMMENT2 values without killing it. + next; + } $content{$_} = ref( $map{$_} ) ? ${ $map{$_} } @@ -208,6 +216,19 @@ STATE => 'state', ZIP => \$zip, # 'zip' with non-alnums removed COUNTRY => 'country', + + # CUSTCODE appears to be cut off at 18 characters and isn't currently + # reportable. It is better to store local customer ids in the COMMENT1/2 + # fields because of that. (As of 8/18/2009.) + CUSTCODE => 'customer_id', + SHIPTOFIRSTNAME => 'ship_first_name', + SHIPTOLASTNAME => 'ship_last_name', + SHIPTOSTREET => 'ship_address', + SHIPTOCITY => 'ship_city', + SHIPTOSTATE => 'ship_state', + SHIPTOZIP => 'ship_zip', + SHIPTOCOUNTRY => 'ship_country', + ); my @required = qw( TRXTYPE TENDER PARTNER VENDOR USER PWD ); @@ -232,6 +253,9 @@ ACCT CVV2 EXPDATE AMT FIRSTNAME LASTNAME NAME EMAIL COMPANYNAME STREET CITY STATE ZIP COUNTRY + SHIPTOFIRSTNAME SHIPTOLASTNAME + SHIPTOSTREET SHIPTOCITY SHIPTOSTATE SHIPTOZIP SHIPTOCOUNTRY + CUSTCODE ) ); @@ -261,8 +285,27 @@ "headers" => \%req_headers, ); + # Payflow Pro does not use URL encoding for the request, so we need to use + # their encoding scheme. We append bracketed numbers to the keys to denote + # lengths for values containing the special characters '&' and '='. + # https_post allows us to pass a scalar rather than a reference to specify + # our own string. There are two sections in the Payflow Pro Developer's + # Guide on this subject: 'Using Special Characters in Values' and 'PARMLIST + # Syntax Guidelines' that give info on this. + my $params_string = join( + '&', + map { + my $key = $_; + my $value = defined( $params{$key} ) ? $params{$key} : ''; + if ( index( $value, '&' ) != -1 || index( $value, '=' ) != -1 ) { + $key = $key . "[" . length($value) . "]"; + } + "$key=$value"; + } keys %params + ); + my ( $page, $resp, %resp_headers ) = - $self->https_post( \%options, \%params ); + $self->https_post( \%options, $params_string ); $self->response_code( $resp ); $self->response_page( $page ); @@ -591,6 +634,20 @@ ZIP => \$zip, # 'zip' with non-alphanumerics removed COUNTRY => 'country', + # CUSTCODE appears to be cut off at 18 characters and isn't currently + # reportable. It is better to store local customer ids in the COMMENT1/2 + # fields because of that. (As of 8/18/2009.) + CUSTCODE => 'customer_id', + + SHIPTOFIRSTNAME => 'ship_first_name', + SHIPTOLASTNAME => 'ship_last_name', + SHIPTOSTREET => 'ship_address', + SHIPTOCITY => 'ship_city', + SHIPTOSTATE => 'ship_state', + SHIPTOZIP => 'ship_zip', + SHIPTOCOUNTRY => 'ship_country', + + The required Payflow Pro parameters for credit card transactions are: TRXTYPE TENDER PARTNER VENDOR USER PWD ORIGID
I think I'll reject this one for now: *) This next one is controversial and I almost took it out myself as I'm not sure it's a perfect idea, since we sort of want the user to follow the BOP API. I'll leave the decision up to you. --- We no longer overwrite values when doing mapping. For example, if a user manually enters COMMENT2, but does not enter the invoice_number, we previously would wipe their COMMENT2 value and not use it. With this patch, we recognize they didn't enter invoice_number and we don't overwrite the COMMENT2 value the user specified. This is the first part of the patch if you want to remove: @@ -139,6 +139,14 @@ I'm not convinced it is good or bad, but I think it is a big enough change and without a good case supporting the change I think we should probably keep things as-is. Feel free to try and "sell" it... if you think you've got a good reason to do this as perhaps I'm missing the point and/or problem still.
released v1.00 with fixes