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

Sending a more explicit header when doing a scan/query from Smoothie #2248

Closed
teolemon opened this issue Jun 10, 2022 · 15 comments · Fixed by #2408
Closed

Sending a more explicit header when doing a scan/query from Smoothie #2248

teolemon opened this issue Jun 10, 2022 · 15 comments · Fixed by #2408

Comments

@teolemon
Copy link
Member

teolemon commented Jun 10, 2022

What

  • Sending a more explicit header when doing a scan/query from Smoothie

At the moment the log looks like this:

xxxx - - [10/Jun/2022:15:02:48 +0200] "POST /cgi/display.pl?api/v0/product/8059070744132.json HTTP/1.0" 200 20617 "-" "Dart/2.17 (dart:io)" 0/54837
  • When a scan it could say Open Food Facts vXXX - Scan
  • When a search it could say Open Food Facts vXXX - Search
@stephanegigandet
Copy link
Contributor

Please also add the platform ios / android

@teolemon teolemon added this to the V1 milestone Jun 10, 2022
@VaiTon
Copy link
Member

VaiTon commented Jun 10, 2022

Can we use User-Agent? @teolemon
Or the uuid like we wanted to do in the Android native app?
openfoodfacts/openfoodfacts-androidapp#4438

@teolemon
Copy link
Member Author

@monsieurtanuki
Copy link
Contributor

You're already supposed to see something like that, isn't that the case?

  UserAgent(
    name: 'Smoothie - ${packageInfo.appName}',
    version: '${packageInfo.version}+${packageInfo.buildNumber}',
    system: Platform.operatingSystemVersion,
    url: 'https://world.openfoodfacts.org/',
  );

Regarding using the user agent for a distinction between scan and search, it would mean changing the user agent for each query (ok, we have the technology for that) while it's supposed to be kind of static.

@monsieurtanuki
Copy link
Contributor

Correction: the "user agent" parameters are supposed to appear as get/post parameters (cf. HttpHelper)

      map['app_name'] = OpenFoodAPIConfiguration.userAgent!.name!;
      map['app_version'] = OpenFoodAPIConfiguration.userAgent!.version!;
      map['app_uuid'] = OpenFoodAPIConfiguration.uuid!;

@monsieurtanuki
Copy link
Contributor

@teolemon Actually looking back at the OP we don't call /cgi/display.pl from off-dart; maybe it's a redirection done on the server where we unfortunately dismiss the POST parameters.

@teolemon
Copy link
Member Author

@monsieurtanuki is that ok for @cli1005 to work on this ? As we get Dart info in the logs, I believe it's a client side issue (eg Smoothie)

@monsieurtanuki
Copy link
Contributor

@teolemon @cli1005 Sure, go ahead!

@cli1005
Copy link
Contributor

cli1005 commented Jun 14, 2022

The issue might be in Open Food Facts API... and the API accepts only app_name, app_version, app_uuid actually, if you want send more informations, like device system(ios/android) or comment (instead of what I did in #2262), we might have to modify the Open Food Facts API as well.

Another question, @teolemon could you please tell me how to find the log as you mentioned in the issue description? thanks

@monsieurtanuki
Copy link
Contributor

monsieurtanuki commented Jun 15, 2022

@cli1005 Indeed. We should probably add something like app_system (e.g. 'android 12').
Beyond that, a more generic optional comment would make sense (e.g. scan). Or specific to barcode search, a source parameter (e.g. scan/search).
Don't know which would be the best solution as I don't see the full picture, but a generic dev_comment would be versatile.

@teolemon teolemon modified the milestones: V1, V1.1 Jun 18, 2022
@cli1005
Copy link
Contributor

cli1005 commented Jun 20, 2022

app_platform and comment have been added into UserAgentParameters (openfoodfacts/openfoodfacts-dart#482).
As monsieurtanuki mentioned, we do not call display.pl in OFF API, among all perl called, we do not sent UserAgentParameters for only 4 following PL:

  • test.pl
  • product_image_upload.pl (is it normal? why?)
  • user.pl
  • reset_password.pl

I could add Scan/ Search in parameter comment will sending in Smoothie, but it might be better to do it in OFF API...

@stephanegigandet
Copy link
Contributor

Regarding using the user agent for a distinction between scan and search, it would mean changing the user agent for each query (ok, we have the technology for that) while it's supposed to be kind of static.

Hi @monsieurtanuki, having the app name + platform + source of query (scan, or search, or list) in the UserAgent is very useful server side as we get the UserAgent in the nginx + Apache logs directly, before we interpret GET or POST parameters.

This is used currently to get scan statistics from our app for products: we just parse nginx logs.

@stephanegigandet
Copy link
Contributor

we do not call display.pl in OFF API,

It's an internal redirect server side, any call to /api/ will be handled by display.pl

@stephanegigandet
Copy link
Contributor

we don't call /cgi/display.pl from off-dart; maybe it's a redirection done on the server where we unfortunately dismiss the POST parameters.

There is redirection server side done by a nginx reverse proxy. This proxy does not analyze the POST parameters, it just passes them along. But it is very useful for us to be able to identify our app, and scans, in this nginx reverse proxy.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Ok then. We go a little beyond the normal use of User Agent, fair enough. A little problem is that the User Agent is static: that made sense to use a static field for characteristics that don't change often (e.g. device or app version), that's a bit lousy for something that is supposed to be changed at every request (e.g. 2 requests in parallel and the second request overwrites the first one's user agent). I guess we can live with that, it's not like one user scans two products at the very same time. With @cli1005's #484. now we have a "comment" field in user agent, so we are ok I guess:

  const UserAgent({
    this.name,
    this.version,
    this.system,
    this.url,
    this.comment,
  });

@cli1005 cli1005 removed their assignment Jun 27, 2022
@monsieurtanuki monsieurtanuki self-assigned this Jun 27, 2022
monsieurtanuki added a commit to monsieurtanuki/smooth-app that referenced this issue Jun 28, 2022
…rcode search

Impacted files:
* `barcode_product_query.dart`: new parameter `isScanned` to distinguish scan and barcode search via the user agent comment field
* `continuous_scan_model.dart`: "this IS a scan"
* `product_dialog_helper.dart`: "this is NOT a scan"
* `product_query.dart`: new method to set the user agent comment field
monsieurtanuki added a commit that referenced this issue Jun 28, 2022
…2408)

Impacted files:
* `barcode_product_query.dart`: new parameter `isScanned` to distinguish scan and barcode search via the user agent comment field
* `continuous_scan_model.dart`: "this IS a scan"
* `product_dialog_helper.dart`: "this is NOT a scan"
* `product_query.dart`: new method to set the user agent comment field
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment