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

Pagination broken - Next URL more than 2048 characters #1567

Open
mikemassa84 opened this issue Jan 5, 2021 · 18 comments
Open

Pagination broken - Next URL more than 2048 characters #1567

mikemassa84 opened this issue Jan 5, 2021 · 18 comments
Assignees
Labels
Bug Bug bug bug. VSTS-Backlog On VSTS Backlog
Milestone

Comments

@mikemassa84
Copy link

Describe the bug
The "next" url in the link section of a Bundle resource results in a 404 response. I believe this is related to the URL specified as "next" is longer than 2048 characters. In our case, it is 5,399 characters. We are using the Azure API for FHIR.

FHIR Version?
R4

Data provider?
CosmosDB

To Reproduce
Steps to reproduce the behavior:

  1. Send a GET response to /Encounter (We have > 100,000 Encounters)
  2. Copy the link of the URL tagged with relation: next in the link object.
  3. Send a GET to the link copied from step 2.
  4. You should get a 404 response if the URL is too long

Expected behavior
Next page of Encounter resources should display

Actual behavior
A 404 response message is returned saying The resource you are looking for has been removed, had its name changed, or is temporarily unavailable.

Here is our initial call to /Encounter:
image

Here is the response when calling the URL from the next link:
image

@mikemassa84 mikemassa84 added the Bug Bug bug bug. label Jan 5, 2021
@feordin feordin self-assigned this Jan 6, 2021
@feordin
Copy link
Contributor

feordin commented Jan 6, 2021

@mikemassa84 Thanks for the bug report. Could you let me know the Azure API for FHIR account name? It will be helpful in trying to debug this.

@mikemassa84
Copy link
Author

@mikemassa84 Thanks for the bug report. Could you let me know the Azure API for FHIR account name? It will be helpful in trying to debug this.

@feordin Name is nma1-s2-fhir
Let me know if you need anything else.

@feordin
Copy link
Contributor

feordin commented Jan 7, 2021

@mikemassa84 I have tried to repro your issue, but have not been able to see the same error which you have shown. We have our service configured to handle Urls of up to 8kb, and Cosmos DB should be generating continuation tokens of up to 4kb. We then encode those continuation tokens in base64, which would match up with the length you see of 5399. When I attempt to query your instance with a url of approximately 5400 bytes in length, I receive an operation outcome with Authentication Failed error, which is what I expect to happen.

Is it possible you have a proxy server running? or other routing mechanism which might apply the 2k limit to the Url length?

Are you able to open an azure support ticket? It might be easier to continue debugging the issue with a more direct method of communication.

@mikemassa84
Copy link
Author

@feordin We do have a proxy in front of our fhir api. It is running as an Azure function and simply passes the request through as is. I'll try stepping through that code and see if the function is truncating the continuation token. If that's not the case, I'll have an Azure support ticket opened. My team also has a standing meeting with Microsoft on Fridays, so maybe we can pull you in that way as well?

@mikemassa84
Copy link
Author

@feordin I ran our fhir proxy function locally and it worked without issue. The failure is occurring when the proxy is running deployed to Azure.

@feordin
Copy link
Contributor

feordin commented Jan 7, 2021

I think it would be a great topic to bring up in your standing meeting on Friday. I saw this documentation (https://docs.microsoft.com/en-us/azure/azure-functions/functions-bindings-http-webhook-trigger?tabs=csharp) related to HTTP triggers for Azure functions which seem to indicate that the limit for a function url length is 4kb. It also seems to indicate that it may be configurable in the web.config file.

@mikemassa84
Copy link
Author

@feordin You can probably close this as the issue does seem related to the Azure functions being limited by maxQueryStringLength="4096" being set. Not sure if we are able to update that, but I'll reach out to support for it.

I imagine the FHIRProxy MS wrote would encounter the same issue?
https://github.com/microsoft/health-architectures/tree/master/FHIR/FHIRProxy

@mikemassa84
Copy link
Author

@mikemassa84 I have tried to repro your issue, but have not been able to see the same error which you have shown. We have our service configured to handle Urls of up to 8kb, and Cosmos DB should be generating continuation tokens of up to 4kb. We then encode those continuation tokens in base64, which would match up with the length you see of 5399. When I attempt to query your instance with a url of approximately 5400 bytes in length, I receive an operation outcome with Authentication Failed error, which is what I expect to happen.

Do you think it would make sense to ensure the base64 encoded token does not exceed 4kb, since that is what is sent back by calling systems?

@CaitlinV39 CaitlinV39 added Feedback-Pending acceptance Item is pending acceptance or more feedback from the submitter Feedback-Discussion Discussion is needed. and removed Feedback-Pending acceptance Item is pending acceptance or more feedback from the submitter labels Jan 11, 2021
@feordin
Copy link
Contributor

feordin commented Jan 12, 2021

@mikemassa84 I have tried to repro your issue, but have not been able to see the same error which you have shown. We have our service configured to handle Urls of up to 8kb, and Cosmos DB should be generating continuation tokens of up to 4kb. We then encode those continuation tokens in base64, which would match up with the length you see of 5399. When I attempt to query your instance with a url of approximately 5400 bytes in length, I receive an operation outcome with Authentication Failed error, which is what I expect to happen.

Do you think it would make sense to ensure the base64 encoded token does not exceed 4kb, since that is what is sent back by calling systems?

@mikemassa84 Apologies for the delay in the response. I wanted to check in with the broader team to confirm making a change. Yes, we agree that making the continuation token smaller is a good idea, and it is on the backlog to be done. I am afraid I don't have an ETA on when it will happen.

I am curious if you were able to change the maxQueryStringLength of the azure function and mitigatge the issue for now?

@feordin feordin closed this as completed Jan 12, 2021
@mikemassa84
Copy link
Author

@feordin Unfortunately, I could not find a way to change the maxQueryStringLength setting. I tried using ISS Manager extension for App Service to change the setting of the underlying IIS, but it didn't seem to make any difference. Either the extension doesn't work, I didn't use it properly, or the extension isn't relevant to function apps.

I was able to get in touch with some of the folks who work on the MS FHIR Proxy, so hopefully I can come up with something with them. Worst case scenario for me is I use my proxy for post/put/patch/delete operations and go directly to the fhir api for read operations.

@CaitlinV39
Copy link
Contributor

CaitlinV39 commented Feb 2, 2021

AB#78736

@CaitlinV39 CaitlinV39 reopened this Mar 10, 2021
@CaitlinV39 CaitlinV39 added VSTS-Current We are working on this item in the current sprint and removed Feedback-Discussion Discussion is needed. labels Mar 10, 2021
@CaitlinV39 CaitlinV39 added this to the S58 milestone Mar 10, 2021
@CaitlinV39 CaitlinV39 assigned Ivanidzo4ka and abhijeett and unassigned feordin Mar 17, 2021
@CaitlinV39 CaitlinV39 modified the milestones: S58, S59 Mar 22, 2021
@CaitlinV39
Copy link
Contributor

We reduced the size of the token to 3K so this should resolve.

@mikemassa84
Copy link
Author

We reduced the size of the token to 3K so this should resolve.

This is still broken if you search by multiple parameters. Here's an example from our server:

/Patient?name=Smith&gender=female

This generated a ct with the following:
W3sidG9rZW4iOiIrUklEOn44d0JSQUtzNzJndERxQThBQUFBQUFBPT0jUlQ6MSNUUkM6MTAjSVNWOjIjSUVPOjY1NTY3I1FDRjo3I0ZQUDpBZ2crQUFBQUFBQUFBRXdEQUFBQU1BQUFQZ0FBQUFBQUFBQUNBRU9vUHdBQUFBQUFBQUFFQUNXSm9wTkJBQUFBQUFBQUFBSUF3NlZIQUFBQUFBQUFBQUlBaXFGSUFBQUFBQUFBQUFJQXBJTkpBQUFBQUFBQUFBSUE1NU5OQUFBQUFBQUFBQUlBTkxKUUFBQUFBQUFBQUFJQU5LdFdBQUFBQUFBQUFBSUFWcmxYQUFBQUFBQUFBQUlBYllOWkFBQUFBQUFBQUFJQUtLUmFBQUFBQUFBQUFBUUEvYko1aEZzQUFBQUFBQUFBQWdDSGcxMEFBQUFBQUFBQUFnQXhtR0VBQUFBQUFBQUFBZ0E0cDJNQUFBQUFBQUFBQkFDR2tNaU5aQUFBQUFBQUFBQUNBTSs2WlFBQUFBQUFBQUFFQUtDTFc1Rm1BQUFBQUFBQUFBSUF4NUpuQUFBQUFBQUFBQVFBUmJ4ZmdXZ0FBQUFBQUFBQUFnQ1RzV2tBQUFBQUFBQUFBZ0JHa1cwQUFBQUFBQUFBQmdCempjU1E4NDl1QUFBQUFBQUFBQUlBc3FKd0FBQUFBQUFBQUFZQSs0TnZnY3V5Y1FBQUFBQUFBQUFDQU5lV2NnQUFBQUFBQUFBSUFLZVBySnVEaDJhRWN3QUFBQUFBQUFBQ0FQS2FkQUFBQUFBQUFBQUNBSG03ZFFBQUFBQUFBQUFFQUdXSDZwWjJBQUFBQUFBQUFBSUFYN0YzQUFBQUFBQUFBQUlBODZSNUFBQUFBQUFBQUFRQTBwSHZnM29BQUFBQUFBQUFDQUNjaEZPWmJvSHBnbnNBQUFBQUFBQUFBZ0NjaTN3QUFBQUFBQUFBQWdDMW5IMEFBQUFBQUFBQUJBQjRnZGlEZ2dBQUFBQUFBQUFDQUdXM2d3QUFBQUFBQUFBSUFCQ0NJbzh6a3NXVWhBQUFBQUFBQUFBSUFDS0s2SUllaEtPbWhRQUFBQUFBQUFBQ0FLbXZoZ0FBQUFBQUFBQUNBRW0waHdBQUFBQUFBQUFHQUFDZEFZdmNoWWdBQUFBQUFBQUFBZ0FabG9rQUFBQUFBQUFBQWdBL2w0MEJBQUFBQUFBQUJnQUZnY0NvK29xT0FRQUFBQUFBQUFJQUZJZVBBUUFBQUFBQUFBUUFESjNHaUpFQkFBQUFBQUFBQkFBQ2taeXRrZ0VBQUFBQUFBQUVBRGFOZEl5VEFRQUFBQUFBQUFZQVlZTWFpSldrbEFFQUFBQUFBQUFFQUdtQ0k3eVZBUUFBQUFBQUFBUUFFWXA1bHBZQkFBQUFBQUFBQkFCdHEwcVBtQUVBQUFBQUFBQUdBSk9GTnA5NW1wa0JBQUFBQUFBQUFnQUZzWm9CQUFBQUFBQUFDQUF6Z1Q2T0xwWGRocHNCQUFBQUFBQUFCQUJjaDF1Z25BRUFBQUFBQUFBQ0FPZWJuUUVBQUFBQUFBQUNBTUtEbndFQUFBQUFBQUFDQUlPNG9BRUFBQUFBQUFBQ0FLeUFvUUVBQUFBQUFBQUNBUCtIb2dFQUFBQUFBQUFFQUNPWHE1S2pBUUFBQUFBQUFBUUFKYW8va0tRQkFBQUFBQUFBQWdBaG1xY0JBQUFBQUFBQUJBQktrMENFcUFFQUFBQUFBQUFFQURlVStLZXBBUUFBQUFBQUFBUUE1NEdlajZvQkFBQUFBQUFBQkFDSXFZbURxd0VBQUFBQUFBQUNBQ1NPclFFQUFBQUFBQUFHQU0yZmlKbFZncTRCQUFBQUFBQUFBZ0JKb2JBQkFBQUFBQUFBQkFEMGpaR0NzUUVBQUFBQUFBQUVBSEtMZ0oreUFRQUFBQUFBQUFJQVVJS3pBUUFBQUFBQUFBWUFHWUQ4aTZhTHRBRUFBQUFBQUFBRUFMQ0JHWisxQVFBQUFBQUFBQUlBVW9xMkFRQUFBQUFBQUFRQWFKWWdtN2NCQUFBQUFBQUFBZ0FLdTdnQkFBQUFBQUFBQ0FBYWhjMkdrWk1BbDdrQkFBQUFBQUFBQkFCbXAvaUN1Z0VBQUFBQUFBQUVBTXEyMTRMQUFRQUFBQUFBQUFJQXU2bkNBUUFBQUFBQUFBSUFITHpEQVFBQUFBQUFBQUlBN2JuRUFRQUFBQUFBQUFRQXA2MlhpY1VCQUFBQUFBQUFDQURSanYrRU00RWNnY1lCQUFBQUFBQUFBZ0ROa3NnQkFBQUFBQUFBQkFCNGpEQ255UUVBQUFBQUFBQUNBTitNeWdFQUFBQUFBQUFHQUdhRVNZUmtzczBCQUFBQUFBQUFCQUFjajk2a3pnRUFBQUFBQUFBR0FNV0VacGpZb2M4QkFBQUFBQUFBQmdEMWh5bVk2WXJTQVFBQUFBQUFBQUlBRnJEVEFRQUFBQUFBQUFRQWlxT0ZsTlFCQUFBQUFBQUFCQUNhbEZpRDFRRUFBQUFBQUFBQ0FPMnIxd0VBQUFBQUFBQUNBQ09tMkFFQUFBQUFBQUFFQUppT3NwSGFBUUFBQUFBQUFBSUEwSkRiQVFBQUFBQUFBQVFBYTVuRWs5NEJBQUFBQUFBQUFnQUFrZDhCQUFBQUFBQUFBZ0QvaXVBQkFBQUFBQUFBQkFBZWorYWM0UUVBQUFBQUFBQUVBQmlabW9MaUFRQUFBQUFBQUFJQVZJcmpBUUFBQUFBQUFBWUF6Wm96a0M2QTVRRUFBQUFBQUFBQ0FHYVg1d0VBQUFBQUFBQUdBRXFJczRFWW9la0JBQUFBQUFBQUFnRHNtK29CQUFBQUFBQUFBZ0IvaytzQkFBQUFBQUFBQkFBM2lXQ3I3QUVBQUFBQUFBQUVBTGFSeTVmdUFRQUFBQUFBQUFJQSs2ZnZBUUFBQUFBQUFBZ0FiWURsallHTWw0dnhBUUFBQUFBQUFBWUFCSjh1Z2FPUzlBRUFBQUFBQUFBR0FDQ0lyb2twcVBjQkFBQUFBQUFBQkFDU2ovbUMrQUVBQUFBQUFBQUVBRFdicjUvNkFRQUFBQUFBQUFJQWxaejhBUUFBQUFBQUFBSUFENlBvQVFBQUFDQUFBQWdBcEtrRmh2MkZMSUxwQVFBQUFDQUFBQW9BY0pkemhGeUNsNE5Wa3VzQkFBQUFJQUFBQkFCTG5uMkM3QUVBQUFBZ0FBQUNBRXlBN1FFQUFBQWdBQUFDQU5LWTdnRUFBQUFnQUFBQ0FCV3k3d0VBQUFBZ0FBQUVBTGlGTHFUd0FRQUFBQ0FBQUFRQTRhZXNodkVCQUFBQUlBQUFDQUNvbmlXR3hveFVnL0lCQUFBQUlBQUFDQUFLZ3dHRCtvTExyL01CQUFBQUlBQUFBZ0JjcS9RQkFBQUFJQUFBQkFDL2hBeU85UUVBQUFBZ0FBQUVBRDZJRUp6MkFRQUFBQ0FBQUFvQUtZU3NnRVNIVVlDRWlmY0JBQUFBSUFBQUJnQk9naldZYjV6NEFRQUFBQ0FBQUFJQVU0djdBUUFBQUNBQUFBUUFaSy9zaGZ3QkFBQUFJQUFBREFCaWdOR0kzNE9Xa1hpQnBKVDlBUUFBQUNBQUFBSUFPNDcrQVFBQUFDQUFBQVlBdW9na24yZVcvd0VBQUFBZ0FBQUNBSjI0QUFJQUFBQWdBQUFFQU1lYVdvQUJBZ0FBQUNBQUFBWUFJNXhRamNLR0FnSUFBQUFnQUFBSUFBS0ZnNUV1bDkyUkF3SUFBQUFnQUFBRUFQZUFZYW9FQWdBQUFDQUFBQVFBam9QV293VUNBQUFBSUFBQUFnQ1Zwd1lDQUFBQUlBQUFBZ0JOcVFnQ0FBQUFJQUFBQkFCZ29qeVdDUUlBQUFBZ0FBQUdBTTZJTlkvZWxBb0NBQUFBSUFBQUJnQ2NnQ0daUkpZTEFnQUFBQ0FBQUFRQUo2WHZqQXdDQUFBQUlBQUFCQURuZ0xpNkRnSUFBQUFnQUFBR0FBYURiTFJVaHc4Q0FBQUFJQUFBQ2dCQWp4ZUhQb3NVbTNtQyIsInJhbmdlIjp7Im1pbiI6IiIsIm1heCI6IjA1QzFBMCJ9fV0%3D

@CaitlinV39
Copy link
Contributor

Thanks @mikemassa84. We are able to reproduce and are considering what the best next step would be. We did reduce the size of the token, but when we base64 encode it, it is adding enough that it is getting pushed over the limit. I'll reopen this issue.

@CaitlinV39 CaitlinV39 reopened this Jun 2, 2021
@CaitlinV39 CaitlinV39 removed the VSTS-Current We are working on this item in the current sprint label Jun 2, 2021
@mikemassa84
Copy link
Author

@CaitlinV39 maybe a hash and store? You could put the base64 string through a sha256 hash and build a hex string out of it. That'd consistently give you a 64 character string you could use in place of the base64 string. You'd have to store the hash to original string mapping though to get back to your original token...

@shahedaAnsari
Copy link

we are also facing same issue and we are using 2.0.172 build

@CaitlinV39 CaitlinV39 added the VSTS-Pending Prioritization We would like to add this to the backlog label Oct 13, 2021
@CraigP68
Copy link

CraigP68 commented Dec 9, 2021

AB#87261

@CraigP68 CraigP68 modified the milestones: S59, backlog Dec 9, 2021
@CraigP68 CraigP68 added VSTS-Backlog On VSTS Backlog and removed VSTS-Pending Prioritization We would like to add this to the backlog labels Dec 9, 2021
@EXPEkesheth
Copy link
Contributor

@mikemassa84 - we recently addressed the issue by introducing header "x-ms-documentdb-responsecontinuationtokenlimitinkb". The next link in the bundle has a continuation token size limit of 3KB. You have flexibility to tweak the continuation token size between 1 to 3KB using header. Please let us know if your issue is addressed with the header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug bug bug. VSTS-Backlog On VSTS Backlog
Projects
None yet
Development

No branches or pull requests

8 participants